pymmcore-plus / useq-schema

An implementation agnostic schema for describing a sequence of events during a multi-dimensional imaging acquisition.
https://pymmcore-plus.github.io/useq-schema/
BSD 3-Clause "New" or "Revised" License
14 stars 5 forks source link

WellPlatePlan points validator #170

Open fdrgsp opened 4 days ago

fdrgsp commented 4 days ago

Hey @tlambert03 , what do you do this here? Is it because the max_width and max_height default value is np.inf?

https://github.com/pymmcore-plus/useq-schema/blob/6bed705d4e5c67838db2913ae8b644b385013a08/src/useq/_plate.py#L167-L177

This will overwrite anything that the user inputs as max_width and max_height...shouldn't it be executed only if max_width and max_height are not provided?

As an example:

from useq import RandomPoints
from useq._plate import WellPlate, WellPlatePlan

rnd = RandomPoints(
        fov_width=512.0,
        fov_height=512.0,
        num_points=3,
        max_width=6400,
        max_height=6400,
        random_seed=3893395329,
        allow_overlap=False,
    )

wp = WellPlatePlan(
    plate=WellPlate(
        rows=8,
        columns=12,
        well_spacing=(9.0, 9.0),
        well_size=(6.4, 6.4),
        name="96-well",
    ),
    a1_center_xy=(-0.0, -0.0),
    rotation=-0.0,
    selected_wells=(
        [0, 0, 0, 1, 1, 1, 2, 2, 2, 4, 5, 6],
        [1, 2, 3, 3, 2, 1, 1, 2, 3, 6, 6, 7],
    ),
    well_points_plan=rnd,
)

assert wp.well_points_plan == rnd
tlambert03 commented 4 days ago

This will overwrite anything that the user inputs as max_width and max_height...shouldn't it be executed only if max_width and max_height are not provided?

yes that would have been better.

The primary goal there was to "relax" the requirements for declaring a random points plan. prior to that PR, you couldn't make a random points plan without providing all of width, height, shape, etc... but doing that in the context of a plate plan felt silly, since the plate itself can already provide all of that information. However, you're absolutely right that someone should be able to manually provide that information and have it not be overridden. Sorry if it took you a while to track that one down 😅

tlambert03 commented 3 days ago

closed by #171 right @fdrgsp?

fdrgsp commented 3 days ago

closed by #171 right @fdrgsp?

Yes sorry I forgot to pin it.