Conversation
3303023 to
59b7c48
Compare
5e87d6a to
96ed3d8
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1896 +/- ##
==========================================
- Coverage 94.62% 89.98% -4.65%
==========================================
Files 215 215
Lines 19266 19335 +69
==========================================
- Hits 18230 17398 -832
- Misses 1036 1937 +901
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
a725cce to
fc98c2b
Compare
e92d58b to
339ef68
Compare
There was a problem hiding this comment.
Thanks @atravitz ! I added some comments regarding validation and error handling.
One thing that's currently missing is that the network planner always adds a SolventComponent to the ChemicalSystem, however the membrane case does not support a SolventComponent in the complex.
Once you add the protocol.validate to the network planner, this would also raise an Error.
42a42bb to
038177b
Compare
hannahbaumann
left a comment
There was a problem hiding this comment.
Thanks @atravitz , this looks great! Left some small comments, the only important one is the catching of the actual error which would be very helpful to users I think.
47a2a03 to
f3d155d
Compare
|
We can wait for the dust to settle but I think we should also include a news item here so it shows up in the change log |
Co-authored-by: Mike Henry <11765982+mikemhenry@users.noreply.github.com>
Co-authored-by: Mike Henry <11765982+mikemhenry@users.noreply.github.com>
68769ea to
2c878d1
Compare
29dd520 to
ae79288
Compare
|
No API break detected ✅ |
|
Maybe add a link in the news item to the membrane prep tutorial? |
Checklist
newsentry, or the changes are not user-facing.pre-commit.ci autofix.Manual Tests: these are slow so don't need to be run every commit, only before merging and when relevant changes are made (generally at reviewer-discretion).
Developers certificate of origin