fix deprecated attribute-initialisation path in Builder (test to be added) #1516
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1516 +/- ##
=======================================
Coverage 85.33% 85.33%
=======================================
Files 379 379
Lines 9332 9332
=======================================
Hits 7963 7963
Misses 1369 1369 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| assert not inspect.isclass(backend) | ||
| self.formulae = backend.formulae | ||
| self.particulator = Particulator(n_sd, backend) | ||
| self.req_attr_names = ["multiplicity", "water mass", "cell id"] |
There was a problem hiding this comment.
Should we change it also here?
There was a problem hiding this comment.
IIUC, the idea here is that we always require the water mass attribute (which might be a derived one) to be available
| ) | ||
| self.request_attribute("volume") | ||
|
|
||
| if ( |
There was a problem hiding this comment.
and maybe it is not needed anymore?
There was a problem hiding this comment.
here, the scenario is as follows:
- user supplies
volume(which is a derived attribute - in principle should not be supplied by the used) - at the same time, the user does not supply
water mass(we should also check ifsigned water massis not supplied!) - we know how to compute mass from volume
- hence, we compute
signed water massand delete thevolumeattribute - for the user to (intuitively) be able to retrieve values of
volume(given it was part of the input), we also request this derived attribute to be among the available ones
So, there is some reason to do it this way, but at the same time, we are also now in a v3.0.0-pre.XXX stage, hence it is OK to change the API and remove such logic - we could just require to provide water mass or signed water mass without any exceptions.
No description provided.