Skip to content
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

Feature: (Soft) Require Solid Density for Substance.solid() #34

Merged
merged 4 commits into from
Sep 9, 2024

Conversation

jlw387
Copy link
Collaborator

@jlw387 jlw387 commented Sep 9, 2024

Various unrealistic behaviors were noticed for solids that resulted from their densities automatically being set to 1.

A simple example would be adding an arbitrary quantity, say 50 grams, of an arbitrary solid substance, say NaCl, to a container. The container would report a volume of 50 mL, but in reality NaCl has a density of around 2.17 g/mL, so the actual volume taken up by the salt would be ~23 mL.

To address these problems, both the Substance() constructor and Substance.solid() have been given an additional parameter, density, to allow for this value to be specified for both currently supported phases of matter (solids and liquids).

However, there are cases where a user may not know the density of a solid, or it may not be required for setting up their experiments (e.g. when the solid is in a very dilute solution, and the concentration in said solution need not be precise). To allow for these cases, Substance.solid() can still be called without providing a density. Doing so will trigger a warning for the user to inform them of the risks associated with not providing a density, which they can either choose to ignore or choose to address by providing a density.

As one might expect, the unit test code responsible for creating solids was refactored to provide solid densities. This broke some of the strict equalities present on many of the assert statements, so a number of these were adjusted to include a call to pytest.approx(). The two example files were also updated to include solid densities for Substance.solid().

Additional cleanup changes that were made to Substance:

  • Unused concentration attribute was removed.
  • Constructor argument value checking was improved - unsupported molecule types will now trigger a ValueError.
  • Constructor now requires a molecular weight and density, both of which must be positive (for now, the door for infinite density and molecular weight has been left open, but this may be removed if deemed appropriate).
  • Repeated code across Substance(), Substance.solid(), and Substance.liquid() was consolidated (the latter two functions simply call the constructor with the appropriate phase of matter).

Additional changes that were missed in previous features:

  • The two example files contained references to the functions Recipe.create_container() and Container.dilute(), both of which were removed in the Container rework feature pull request. These examples were updated to reflect the current state of the library.
  • The default enzyme density parameter was not removed from either the Config class or pyplate.yaml. The parameter has now been removed from both files.

After some consideration, the option to avoid specifying density for solids has been added back in. However, this will now trigger a warning to alert users to the consequences of not specifying a density.
This included both adding density parameters for the created solids as well addressing changes made from the last feature merge concerning Recipes and Containers. Specifically, Recipe.create_container() calls were removed, and Container.dilute() calls were replaced with Container.dilute_in_place()
@jlw387 jlw387 self-assigned this Sep 9, 2024
@jlw387
Copy link
Collaborator Author

jlw387 commented Sep 9, 2024

Checks have passed, so this branch will now be merged and deleted.

@jlw387 jlw387 merged commit b4f89ec into development Sep 9, 2024
3 checks passed
@jlw387 jlw387 deleted the feature_solid_density branch September 9, 2024 23:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant