Repository navigation
Add starstedt 2ml tube - #1624
Conversation
| ### Height–volume calibration | ||
|
|
||
| Liquid heights were measured with micrometers from the outside base after filling the tube with | ||
| the volumes below. PyLabRobot's height–volume functions use height above the **cavity floor**, so | ||
| the calibration subtracts the 1 mm cavity-floor offset from each measurement. Height above the | ||
| cavity floor increases with volume; it is not a distance measured down from the top rim. |
There was a problem hiding this comment.
this is generic in PLR, we shouldn't have documentation for each individual definition (unless it deviates)
There was a problem hiding this comment.
I removed the general instructions and examples. I kept the tube dimensions and the measured volume range.
| self.assertEqual(restored.get_size_x(), 10.8) | ||
| self.assertEqual(restored.get_size_y(), 10.8) | ||
| self.assertEqual(restored.get_size_z(), 45.0) | ||
| self.assertEqual(restored.max_volume, 2_000) | ||
| self.assertEqual(restored.material_z_thickness, 1.0) | ||
| self.assertEqual(restored.get_anchor(z="cavity_bottom").z, 1.0) | ||
| self.assertTrue(restored.supports_compute_height_volume_functions()) |
There was a problem hiding this comment.
I removed the repeated test. The common container tests already check the save and load functions.
|
thanks Harley! 2 small comments above can you also make sure the ci is all green? |
| _MEASUREMENTS = ( | ||
| (50.0, 3.55), | ||
| (100.0, 4.12), | ||
| (200.0, 7.20), | ||
| (300.0, 9.61), | ||
| (400.0, 11.29), | ||
| (500.0, 13.35), | ||
| (600.0, 15.10), | ||
| (700.0, 16.81), | ||
| (800.0, 18.63), | ||
| (900.0, 20.62), | ||
| (1000.0, 22.37), | ||
| (1500.0, 31.55), | ||
| (2000.0, 40.36), |
There was a problem hiding this comment.
I dont understand, are these new data?
we shouldn't need any tests at all because it's a normal Tube instance just with particular numbers. to clarify from previous comment, we test the implementation of Tube independent of a definition (im adding this to a new agent instruction)
There was a problem hiding this comment.
Aren't these the height_volume_data argument?
Which is part of the definition of a Container.
But it should be a dict and added directly into the defintion function
There was a problem hiding this comment.
Ohhh I see... this is in tests, yes that should not be part of the test suite :)
There was a problem hiding this comment.
Hi Rick and Cam. These are the measured height–volume data for the Sarstedt tube. PLR uses its existing interpolation code with these values.
I removed the extra software tests and placed the measurement dictionary directly inside the tube definition function as height_volume_data. All 13 measured values are unchanged.
There was a problem hiding this comment.
Perfect :)
Please dont forget the 0.0 : 0.0
There was a problem hiding this comment.
why do we require 0: 0? isnt that stupid? let's just add it as a default in the interpolation algorithms
har1eyk
left a comment
There was a problem hiding this comment.
The tests and code checks now pass. The ownership check shows eight files that need approval. Could you review the latest changes and approve them when ready?
|
see previous comment: #1624 (comment) |
|
thanks! |
This PR adds the 2mL skirted tube. These tubes are popular and used by manufacturers like IDT DNA to ship oligos.
Product-Specification-Screw-cap-micro-tube-2-ml-72.609.300.pdf
Height<=> volume calculations have also been included and verified on the Starlet.