add `size_space` to `pygfx.PointsMaterial` scatter kwargs by FlynnOConnell · Pull Request #689 · fastplotlib/fastplotlib

FlynnOConnell · 2025-01-07T20:29:39Z

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

FlynnOConnell · 2025-01-07T20:40:27Z

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

kushalkolar · 2025-01-07T20:45:00Z

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

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.

…ygfx.CoordSpace

kushalkolar

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)

fastplotlib/graphics/scatter.py

fastplotlib/graphics/line.py

…o CoordSpace in docstring

fastplotlib/graphics/_features/_positions_graphics.py

fastplotlib/graphics/_positions_base.py

FlynnOConnell · 2025-01-09T15:47:48Z

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

tests/test_positions_graphics.py

kushalkolar · 2025-01-09T22:18:15Z

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

kushalkolar · 2025-01-09T22:24:16Z

black is still unhappy, just do black fastplotlib/

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

FlynnOConnell · 2025-01-09T22:32:39Z

plz tell black to stop complaining

FlynnOConnell · 2025-01-09T22:55:56Z

@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

kushalkolar · 2025-01-09T23:05:32Z

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.

add size_space to pygfx.PointsMaterial kwargs

f64ff43

FlynnOConnell added 4 commits January 7, 2025 16:26

add CoordSpace graphic feature

7edc007

store _coord_space graphic feature

83f3c17

add coordspace to features init

113ff68

make size_space a property of PositionsGraphic

e54b928

kushalkolar requested changes Jan 7, 2025

View reviewed changes

FlynnOConnell added 4 commits January 7, 2025 17:17

rename CoordSpace->SizeSpace for consistency / avoid confusion with p…

77f4c40

…ygfx.CoordSpace

fix size_space feature, FeatureEvent type=size_space??

9ff2abc

add getters/setters to scatter/line graphics

b398b72

add size_space to _features cls dict

3e527cc

kushalkolar requested changes Jan 7, 2025

View reviewed changes

fastplotlib/graphics/scatter.py Outdated Show resolved Hide resolved

fastplotlib/graphics/scatter.py Show resolved Hide resolved

fastplotlib/graphics/line.py Outdated Show resolved Hide resolved

fastplotlib/graphics/line.py Outdated Show resolved Hide resolved

FlynnOConnell added 2 commits January 7, 2025 18:27

move size_space properties to base PositionGraphics class, add link t…

7e88a08

…o CoordSpace in docstring

fix comment for SizeSpace scatter/line

b8a9be3

kushalkolar requested changes Jan 8, 2025

View reviewed changes

fastplotlib/graphics/_features/_positions_graphics.py Outdated Show resolved Hide resolved

fastplotlib/graphics/_positions_base.py Outdated Show resolved Hide resolved

fastplotlib/graphics/_positions_base.py Outdated Show resolved Hide resolved

FlynnOConnell added 2 commits January 9, 2025 10:21

return .value, set_value()

8a0b9be

don't coerce str, check if "Line" in class name

5db629b

add test for size_space

07de27b

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

kushalkolar requested changes Jan 9, 2025

View reviewed changes

tests/test_positions_graphics.py Show resolved Hide resolved

add tests for setting properties

68c83eb

kushalkolar requested changes Jan 9, 2025

View reviewed changes

tests/test_positions_graphics.py Outdated Show resolved Hide resolved

double quotes for the linting overlords

000d6a3

regen api docs

613ece7

double quotes on "size_space"

38c1897

black formatting spaces / trailing commas

18b3455

kushalkolar approved these changes Jan 9, 2025

View reviewed changes

kushalkolar merged commit 962c9ba into fastplotlib:main Jan 9, 2025

kushalkolar mentioned this pull request Feb 20, 2025

replace weird quotes, update GraphicMethodsMixin #735

Merged

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

Conversation

FlynnOConnell commented Jan 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

FlynnOConnell commented Jan 7, 2025

Uh oh!

kushalkolar commented Jan 7, 2025

Uh oh!

kushalkolar left a comment

Choose a reason for hiding this comment

Uh oh!

kushalkolar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

FlynnOConnell commented Jan 9, 2025

Uh oh!

Uh oh!

Uh oh!

kushalkolar commented Jan 9, 2025

Uh oh!

kushalkolar commented Jan 9, 2025

Uh oh!

FlynnOConnell commented Jan 9, 2025

Uh oh!

FlynnOConnell commented Jan 9, 2025

Uh oh!

kushalkolar commented Jan 9, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

FlynnOConnell commented Jan 7, 2025 •

edited

Loading