biocore / LabControl

lab manager for plate maps and sequence flows
BSD 3-Clause "New" or "Revised" License
2 stars 15 forks source link

Plates > Normalize gDNA Plates arbitrarily selects which quantification to use #214

Closed AmandaBirmingham closed 6 years ago

AmandaBirmingham commented 6 years ago

The Labman database supports storing multiple quantifications per well. However, the existing code for normalizing gDNA plates does not allow the user to decide which of the existing quantifications for the plate to use; instead, it always picks the first one it retrieves from an unsorted SQL query: screen shot 2018-03-14 at 11 50 43 am

In the above example, it picks quantification process 8, which happens to be the most recent one for the plate in question. However, as far as I know, Postgres does not impose any default sorting on query results unless one explicitly uses an order by clause (https://stackoverflow.com/a/6586018) , so I don't think it is safe to assume that the most recent quantification will always be the one returned. This could be very bad if the reason that multiple quantifications were done is that the first one was incorrect.

tanaes commented 6 years ago

Thanks for catching this, @AmandaBirmingham! It looks like we will need to adjust the interfaces to allow selection of plate quantification. I guess this means adding a dropdown to the interfaces that use this, and adding a function to return a list of available quants to the list?

AmandaBirmingham commented 6 years ago

@taenes The main question I have about having users pick the quantification to use is: do we currently store the information they would need to know to make an informed choice? Processes (including quantification) have only three pieces of descriptive information stored with them:

1) The process type (for example, "quantification") 2) The date on which the process was run (aside: this should actually be a timestamp, not a date; see issue #215) 3) The login name of the user who ran the process

Is that going to be enough to allow end users to make an informed decision about which quantification to use?

tanaes commented 6 years ago

Perhaps a 'notes' entry in the quantification process table would be the thing here?

I think it would also be worth defaulting to the most recent quantification. We definitely do re-quantify plates, especially after long periods of time.

AmandaBirmingham commented 6 years ago

@tanaes @ElDeveloper Here is the interface I'm proposing for selecting which quantification to use for normalization: screen shot 2018-04-26 at 1 29 47 pm

As you can see, it uses a table (like that used in the plate list, plate search, etc) to display the available quantitations for each plate; however, instead of a checkbox for each table entry, there is a radio button--because one and only one quantitation must be selected for each plate. The most recent quantitation (as determined by the date of its process) is selected by default.

This is not a mock-up: I've written the necessary code to get us to this point (but not the code to then capture the selected quantitation ids from the interface and pass them to the back end). Please let me know as soon as you can if you have any concerns about this interface design; unless I hear some hollering today, I will move on to implementing the capture-to-back-end functionality :)

AmandaBirmingham commented 6 years ago

Sorry, please excuse the very wrong screenshot--have corrected :(

ElDeveloper commented 6 years ago

@AmandaBirmingham, I think this looks great!

From chatting with @tanaes it seems like we'll rarely have more than half a dozen quantifications, therefore I would suggest that when implementing this, the pagination controls, the "show" dropdown, and the search bar are all hidden. 👍

tanaes commented 6 years ago

Agreed with @ElDeveloper, I think if we can have a PR with this we can quickly solve!

tanaes commented 6 years ago

...also, we think that we should be able to use this same interface mechanics to select the normalizations for a plate to view for issue #195. Does that seem sensible?

AmandaBirmingham commented 6 years ago

I have removed the pagination controls, the "show" dropdown, and the search bar--that part was easy peasy :) And I've got the return of the chosen quantification process to the back-end during normalization working, so all with normalization seems copacetic.

But (there's always a but!) ... thanks to the unit tests, I realize that there's ANOTHER place that quantification information is used: in "Pool library plates"! I don't think this is a problem, per se; as far as I can see, it means that I need to extend labman/gui/templates/library_pooling.html with the same quantification-selection interface & back-end hook I put into normalization.html (above). @tanaes @ElDeveloper : If you have any concerns with me extending library pooling this way, please comment ASAP. Unless I hear from you, I will move ahead with it.

ElDeveloper commented 6 years ago

I am not aware of any problems with extending library_pooling.html, but I'll defer to @tanaes, since he understands the process better.

On (Apr-28-18| 0:48), Amanda Birmingham wrote:

I have removed the pagination controls, the "show" dropdown, and the search bar--that part was easy peasy :) And I've got the return of the chosen quantification process to the back-end during normalization working, so all with normalization seems copacetic.

But (there's always a but!) ... thanks to the unit tests, I realize that there's ANOTHER place that quantification information is used: in "Pool library plates"! I don't think this is a problem, per se; as far as I can see, it means that I need to extend labman/gui/templates/library_pooling.html with the same quantification-selection interface & back-end hook I put into normalization.html (above). @tanaes @ElDeveloper : If you have any concerns with me extending library pooling this way, please comment ASAP. Unless I hear from you, I will move ahead with it.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/jdereus/labman/issues/214#issuecomment-385126652

tanaes commented 6 years ago

Yes, this should be exactly right! Thanks for catching!