-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add well-plate model #166
Conversation
tlambert03
commented
Jun 28, 2024
•
edited
Loading
edited
@fdrgsp, I touched this up a bit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ❤️ it! I belive there are all the things I used in pymmcore-plus/pymmcore-widgets#304 but definitely with a better structure. I will try to use this new WellPlatePlan
in the HCS Wizard
and have a better idea!
# TODO: note that all positions within the same well will currently have the | ||
# same name. This could be improved by modifying `Position.__add__` and | ||
# adding a `name` to GridPosition. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I think this makes sense. We can add a name
to the GridPosition
which can be by default a int
created in the __iter__
method of the RandomPoints
and the iter_grid_positions
of _GridPlan
. So then we can have A1_0000, A1_0001, etc...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Position
__add__
could look something like this:
def __add__(self, other: "Position | GridPosition") -> "Position":
"""Add two positions together to create a new position."""
if isinstance(other, GridPosition) and not other.is_relative:
raise ValueError("Cannot add a non-relative GridPosition to a Position")
other_name = getattr(other, "name", "")
if other_name:
if other_name.isdigit():
other_name = str(other_name).zfill(4)
other_name = f"_{other_name}"
other_z = getattr(other, "z", None)
return Position(
x=self.x + other.x if self.x is not None and other.x is not None else None,
y=self.y + other.y if self.y is not None and other.y is not None else None,
z=self.z + other_z if self.z is not None and other_z is not None else None,
name=f"{self.name}{other_name}" if self.name and other_name else self.name,
sequence=self.sequence,
)
GridPosition
will have the name
attribute:
class GridPosition(NamedTuple):
x: float
y: float
row: int
col: int
is_relative: bool
name: str
the iter_grid_positions
method of _GridPlan
could be like this:
def iter_grid_positions(
self,
fov_width: float | None = None,
fov_height: float | None = None,
*,
mode: OrderMode | None = None,
) -> Iterator[GridPosition]:
"""Iterate over all grid positions, given a field of view size."""
_fov_width = fov_width or self.fov_width or 1.0
_fov_height = fov_height or self.fov_height or 1.0
mode = self.mode if mode is None else OrderMode(mode)
dx, dy = self._step_size(_fov_width, _fov_height)
rows = self._nrows(dy)
cols = self._ncolumns(dx)
x0 = self._offset_x(dx)
y0 = self._offset_y(dy)
for idx, (r, c) in enumerate(_INDEX_GENERATORS[mode](rows, cols)):
yield GridPosition(
x0 + c * dx, y0 - r * dy, r, c, self.is_relative, f"{idx}"
)
and __iter__
form RandomPoints
:
def __iter__(self) -> Iterator[GridPosition]: # type: ignore
seed = np.random.RandomState(self.random_seed)
func = _POINTS_GENERATORS[self.shape]
n_points = max(self.num_points, MIN_RANDOM_POINTS)
points: list[Tuple[float, float]] = []
for idx, (x, y) in enumerate(
func(seed, n_points, self.max_width, self.max_height)
):
if (
self.allow_overlap
or self.fov_width is None
or self.fov_height is None
or _is_a_valid_point(points, x, y, self.fov_width, self.fov_height)
):
yield GridPosition(x, y, 0, 0, True, f"{idx}")
points.append((x, y))
if len(points) >= self.num_points:
break
else:
warnings.warn(
f"Unable to generate {self.num_points} non-overlapping points. "
f"Only {len(points)} points were found.",
stacklevel=2,
)
so, when we iter through the WellPlatePlan
we can have:
x=99.97823292699483 y=197.6533528400231 name='A1_0000'
x=100.89011127568732 y=201.85455673194744 name='A1_0001'
x=98.65280352634389 y=201.43210273330388 name='A1_0002'
x=108.97823292699483 y=197.6533528400231 name='A2_0000'
x=109.89011127568732 y=201.85455673194744 name='A2_0001'
x=107.65280352634389 y=201.43210273330388 name='A2_0002'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm fine with the outcome (A1_0001
) but having the zfill inside of __add__
mixes concerns of something it shouldn't care about. If grid plan __iter__
wants to zfill its own name, that's fine, but all add should do is add the names together or not
Co-authored-by: federico gasparoli <70725613+fdrgsp@users.noreply.github.com>
@fdrgsp, I just added a way to register plates: it takes either kwargs, or a mapping, and values can either be a dict, or a WellPlate instance useq.register_well_plates(
{
"silly": useq.WellPlate(
rows=1, columns=1, well_spacing=1, circular_wells=False
)
},
myplate={"rows": 8, "columns": 8, "well_spacing": 9},
) so pymmcore widgets could load a user database, and then register it with useq |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #166 +/- ##
==========================================
- Coverage 98.28% 95.70% -2.58%
==========================================
Files 14 15 +1
Lines 874 1048 +174
==========================================
+ Hits 859 1003 +144
- Misses 15 45 +30 ☔ View full report in Codecov by Sentry. |
shall we merge it and improve in later PRs @fdrgsp? |
Yes, I think so. We can add the better position naming after (#166 (comment)). I'm working on using this |
yeah, and #169 was also partially with naming in mind (though it's probably not immediately obvious) |