← 返回首页
add `size_space` to `pygfx.PointsMaterial` scatter kwargs by FlynnOConnell · Pull Request #689 · fastplotlib/fastplotlib · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Resetting focus

add size_space to pygfx.PointsMaterial scatter kwargs#689

Merged
kushalkolar merged 19 commits into
fastplotlib:mainfrom
FlynnOConnell:feat-size-space-coords
Jan 9, 2025
Merged

add size_space to pygfx.PointsMaterial scatter kwargs#689
kushalkolar merged 19 commits into
fastplotlib:mainfrom
FlynnOConnell:feat-size-space-coords

Conversation

FlynnOConnell commented Jan 7, 2025
edited
Loading

Copy link
Copy Markdown
Collaborator

The size_space property of PointsMaterial controls which coordinate space to use for the sizing of each point. Default is screen, which using logical pixel values mapped to the screen. Zooming into a plot causes the thickness of circles to remain the same thickness. see here

This aims to allow the other options of CoordSpace Enum to be used instead (world, model, screen).

Copy link
Copy Markdown
Collaborator Author

I'm not immediately seeing the need for a GraphicFeature here, but I could be wrong?

Copy link
Copy Markdown
Member

All aspects of a graphic are managed by a GraphicFeature class and exposed to the user as properties: https://www.fastplotlib.org/ver/dev/developer_notes/graphics.html#graphic-properties

Lines also have a thickness_space parameter so this feature can actually be settable with a property of the PositionsGraphic base class.

kushalkolar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

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.

kushalkolar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Will also need a simple test when you're done, see this for an outline:

@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)

Comment thread fastplotlib/graphics/scatter.py Outdated Show resolved Hide resolved
Comment thread fastplotlib/graphics/scatter.py Show resolved Hide resolved
Comment thread fastplotlib/graphics/line.py Outdated Show resolved Hide resolved
Comment thread fastplotlib/graphics/line.py Outdated Show resolved Hide resolved
Comment thread fastplotlib/graphics/_features/_positions_graphics.py Outdated Show resolved Hide resolved
Comment thread fastplotlib/graphics/_positions_base.py Outdated Show resolved Hide resolved
Comment thread fastplotlib/graphics/_positions_base.py Outdated Show resolved Hide resolved

Copy link
Copy Markdown
Collaborator Author

Eye test looks like everything checks out. will work on a test case next

FlynnOConnell marked this pull request as ready for review January 9, 2025 16:43
FlynnOConnell requested a review from clewis7 as a code owner January 9, 2025 16:43
Comment thread tests/test_positions_graphics.py Show resolved Hide resolved
Comment thread tests/test_positions_graphics.py Outdated Show resolved Hide resolved

Copy link
Copy Markdown
Member

Since you've added/changed the API you will need to regenerate the API docs:

in docs/source:

delete the api dir

with the env activated, run:

python generate_api.py

add and commit docs/api

Copy link
Copy Markdown
Member

black is still unhappy, just do black fastplotlib/

only black format the fastplotlib source dir, not the examples or tests etc.

Copy link
Copy Markdown
Collaborator Author

plz tell black to stop complaining

Copy link
Copy Markdown
Collaborator Author

@kushalkolar I don't even have a guess for why the docs failed

Run SamKirkland/FTP-Deploy-Action@v4.3.5 Error: Error: Input required and not supplied: server

Copy link
Copy Markdown
Member

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.

kushalkolar merged commit 962c9ba into fastplotlib:main Jan 9, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Footer

© 2026 GitHub, Inc.