-
Notifications
You must be signed in to change notification settings - Fork 119
molecular devices plate readers #715
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?
Conversation
| @dataclass | ||
| class MolecularDevicesDataCollection: | ||
| """A collection of MolecularDevicesData objects from multiple reads.""" | ||
|
|
||
| container_type: str | ||
| reads: List["MolecularDevicesData"] | ||
| data_ordering: str | ||
|
|
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.
this class does not add anything because the container type is specified by the user and the data ordering is universal in PLR
it would be better for the read_* functions to return the 2d list that other PLR plate readers do so protocols can be more hardware agnostic
(other PLR plate readers so far do not support / don' have multiple wavelengths implemented, so we might need to make an update to the PlateReader standard)
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.
container type is something returned from the instrument. subclasses of this class also contains all wavelengths used for measurement. these classes are written to store all the parsed data but some of the fields may not be necessary for end users.
do you think we should update the plate reader frontend to have lists of wavelengths as parameters? should we also include kinetic and spectrum modes (common for most plate reads) in the frontend? alternatively, i can write wrappers in my backend that only do endpoint measurements and return 2d lists
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.
container type is something returned from the instrument.
is it new information? i thought the user specifies it when reading
subclasses of this class also contains all wavelengths used for measurement.
these are specified by the user when reading so don't need to be returned back
do you think we should update the plate reader frontend to have lists of wavelengths as parameters?
yes. I can work on this in a small separate PR that I merge before this one because I will need to update the clariostar and cytation backends as well.
what do you think is a good data format? maybe dicts?
>>> pr.read_absorbance(wavelenghts=[300, 400])
{
300: [...], # 2d optional float array
400: [...], # 2d optional float array
}should we also include kinetic and spectrum modes (common for most plate reads) in the frontend?
this changes the output data format right? or can it still be a single 2d list per wavelength? in the case it changes maybe they should be separate functions on both the frontend and backend?
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 agree with you. we can drop container type and all wavelengths.
what about the temperature at which the plate is read? Set point isn't always exactly the same as the actual temp.
what about returning a list of dicts?
>>> pr.read_absorbance(wavelengths=[300,400], kinetic_interval=5, kinetic_num_reads=3) [ {'wavelength':300, 'time':'2015-08-23 18:12', 'temp':37, 'data': [...]}, {'wavelength':400, 'time':'2015-08-23 18:12', 'temp':37, 'data': [...]}, {'wavelength':300, 'time':'2015-08-23 18:13', 'temp':37, 'data': [...]}, {'wavelength':400, 'time':'2015-08-23 18:13', 'temp':37, 'data': [...]}, ... ]
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.
what do you think? should i implement this?
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.
but maybe list of list of dicts:
[intervals][wavelengths for this interval]["time","data","wavelength"]
or
list of dicts where the list is intervals and the data for each interval is keyed by the wavelength so it's easier to access?
[
{
600: {"data": [...], "temp": ..., "time": ...},
700: {"data": [...], "temp": ..., "time": ...},
}
]
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.
temp and time shouldn't be backend specific. most plate readers allow temperature settings and kinetic reads should always have time associated to be useful.
the problem with using wavelength as key is that fluorescence measurments could have different combinations of ex/em wavelengths not just a single one.
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.
let's assume date+temp are universal, I would still prefer this format
every wavelength is read at every interval right?
the problem with using wavelength as key is that fluorescence measurments could have different combinations of ex/em wavelengths not just a single one.
we could have a tuple as a key?
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 think tuple as a key is fine.
[
{ # timepoint0
(ex0,em0): {"data": [...], "temp": ..., "time": ...},
(ex1,em1): {"data": [...], "temp": ..., "time": ...},
},
{ # timepoint1
(ex0,em0): {"data": [...], "temp": ..., "time": ...},
(ex1,em1): {"data": [...], "temp": ..., "time": ...},
},
]
how about that?
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.
Looks great!
|
also, very important, please update the list of supported machines! https://github.com/PyLabRobot/pylabrobot/blob/main/docs/user_guide/machines.md |
1675cd6 to
2b2f7f9
Compare
molecular devices spectramax plate readers: m5 and 384plus