BUG: fix a crash with certain combinations of kwargs for adding dimensionless derived fields to datasets #4638
Conversation
neutrinoceros
added
bug
api-consistency
neutrinoceros
force-pushed
the
hotfix_dimensionless_field
branch
2 times, most recently
from
f94e701 to
509041a
Compare
neutrinoceros
force-pushed
the
hotfix_dimensionless_field
branch
from
509041a to
8866d24
Compare
…sionless derived fields to datasets
neutrinoceros
force-pushed
the
hotfix_dimensionless_field
branch
from
8866d24 to
46025f2
Compare
| @@ -266,7 +277,7 @@ def _generate_fields(self, fields_to_generate): | |||
| fi.name, | |||
| sunits or "dimensionless", | |||
| ) | |||
| elif fi.dimensions != dimensions: | |||
| elif not dimensions_compare_equal(fi.dimensions, dimensions): | |||
There was a problem hiding this comment.
I think this is the right thing to do. However, I will note that there's the possibility that this will add on a non-negligible amount of time to dataset instantiation during field detection. I think that is unlikely, but it is a possibility.
There was a problem hiding this comment.
I'll keep it in mind.
I also want to note that field detection is time-consuming already: in tests that create a small dataset and immediately throw it away, it's adds a couple hundreds ms to each test. I think it'd be a good idea to make it lazy in the future.
…ns of kwargs for adding dimensionless derived fields to datasets
|
I've thought about this, and I can't yet convince myself that losing the full list of available fields is the right trade-off. |
|
Who said anything about loosing it ? What I'm suggesting is just to only evaluate it when and if requested. |
close #4636
Opening as a draft with just the tests, patch incoming.