GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — add `size_space` to `pygfx.PointsMaterial` scatter kwargs by FlynnOConnell · Pull Request #689 · fastplotlib/fastplotlib Conversation
|
|
I'm not immediately seeing the need for a |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
All aspects of a graphic are managed by a Lines also have a |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
A good start!
This doesn't need to be specific to scatters, pygfx line material also uses a thickness_space property that takes the same args and does an analogous thing so this can be in the PositionsGraphic base class.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Will also need a simple test when you're done, see this for an outline:
fastplotlib/tests/test_positions_graphics.py
Lines 419 to 445 in f204f55
| @pytest.mark.parametrize("thickness", [None, 0.5, 5.0]) | |
| def test_thickness(thickness): | |
| fig = fpl.Figure() | |
| kwargs = dict() | |
| for kwarg in ["thickness"]: | |
| if locals()[kwarg] is not None: | |
| # add to dict of arguments that will be passed | |
| kwargs[kwarg] = locals()[kwarg] | |
| data = generate_positions_spiral_data("xy") | |
| graphic = fig[0, 0].add_line(data=data, **kwargs) | |
| if thickness is None: | |
| thickness = 2.0 # default thickness | |
| assert isinstance(graphic._thickness, Thickness) | |
| assert graphic.thickness == thickness | |
| assert graphic.world_object.material.thickness == thickness | |
| if thickness == 0.5: | |
| assert isinstance(graphic.world_object.material, pygfx.LineThinMaterial) | |
| else: | |
| assert isinstance(graphic.world_object.material, pygfx.LineMaterial) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Eye test looks like everything checks out. will work on a test case next |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Since you've added/changed the API you will need to regenerate the API docs: in delete the with the env activated, run:
add and commit |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
black is still unhappy, just do only black format the |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@kushalkolar I don't even have a guess for why the docs failed
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I think it's trying to access the credentials which are repo secrets and it won't since you're on a fork, we'll figure out later how to make docs upload from forks. Anyways the docs built without issues so that's fine and everything else LGTM. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.