-
Notifications
You must be signed in to change notification settings - Fork 8
feat: add possibility to simulate images in batches #95
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
base: main
Are you sure you want to change the base?
Changes from 16 commits
adc3ebd
7b51271
af726d4
c348a82
7887206
d27a44e
f003f39
a23e079
86a20e6
381a95c
e416deb
38954bb
6da8278
9b02463
a9ae073
f468461
2dd3b77
f1d9f71
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -6,7 +6,7 @@ | |||||||||||||||
| import numpy as np | ||||||||||||||||
| import pandas as pd | ||||||||||||||||
| import xarray as xr | ||||||||||||||||
| from pydantic import AfterValidator, Field, model_validator | ||||||||||||||||
| from pydantic import AfterValidator, Field, field_validator, model_validator | ||||||||||||||||
|
|
||||||||||||||||
| from microsim._data_array import ArrayProtocol, from_cache, to_cache | ||||||||||||||||
| from microsim.util import microsim_cache | ||||||||||||||||
|
|
@@ -53,7 +53,7 @@ class Simulation(SimBaseModel): | |||||||||||||||
|
|
||||||||||||||||
| truth_space: Space | ||||||||||||||||
| output_space: Space | None = None | ||||||||||||||||
| sample: Sample | ||||||||||||||||
| samples: Sample | list[Sample] | ||||||||||||||||
| modality: Modality = Field(default_factory=Widefield) | ||||||||||||||||
| objective_lens: ObjectiveLens = Field(default_factory=ObjectiveLens) | ||||||||||||||||
| channels: list[OpticalConfig] = Field(default_factory=lambda: [FITC]) | ||||||||||||||||
|
|
@@ -93,6 +93,28 @@ def _resolve_spaces(self) -> "Self": | |||||||||||||||
| self.output_space.reference = self.truth_space | ||||||||||||||||
| return self | ||||||||||||||||
|
|
||||||||||||||||
| @model_validator(mode="after") | ||||||||||||||||
| def _check_fluorophores_equal_in_samples(self) -> "Self": | ||||||||||||||||
| fp_names = [{lbl.fluorophore.name for lbl in s.labels} for s in self.samples] | ||||||||||||||||
| if len({frozenset(s) for s in fp_names}) != 1: | ||||||||||||||||
| raise ValueError( | ||||||||||||||||
| "All samples in the batch must use the same set of fluorophores." | ||||||||||||||||
| ) | ||||||||||||||||
|
Comment on lines
+99
to
+103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. curious about this. I can see that this would be necessary in the case where we assume that the sample "axis" is just an extension of a non-jagged array. But, maybe sample should be special in that case, and maybe samples should always be iterated over in a for-loop rather than processed in a vectorized fashion? (might help with memory bloat too) that can be decided later. It's definitely easier to start with this restriction and relax it later. |
||||||||||||||||
| return self | ||||||||||||||||
|
|
||||||||||||||||
| @field_validator("samples") | ||||||||||||||||
| def _samples_to_list(value: Sample | list[Sample]) -> list[Sample]: | ||||||||||||||||
| return [value] if isinstance(value, Sample) else value | ||||||||||||||||
|
||||||||||||||||
| @field_validator("samples") | |
| def _samples_to_list(value: Sample | list[Sample]) -> list[Sample]: | |
| return [value] if isinstance(value, Sample) else value | |
| @field_validator("samples", mode="before") | |
| @classmethod | |
| def _samples_to_list(cls, value: Any) -> Sequence[Any]: | |
| return [value] if not isinstance(value, (list, tuple)) else value |
- makes it a classmethod for clarity (pydantic would do this anyway)
- adds the cls, argument to the signature
- because it's a "before" validator, all we really need to do is make sure we're passing a list to pydantic's "actual" validation, which will cast everything to a Sample
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 suppose we don't have to worry about the case where there are zero samples right? sample was previously a required field, so there should always be a sample?
If that's the case, we might want to explicitly require that in the samples field definition. That can be done by using annotated-types
samples: Annotated[list[Sample], MinLen(1)]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 totally agree with your point. However, I don't fully get the relation with the lines you pointed your comment to
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.
it's best in models to avoid unions (except perhaps in the case of
Optional). If one type can serve both purposes, just use that one. Here we can avoid having to checkif isinstance(samples, list)everywhere else in the code. And your field_validator is casting a single object anyway