Open vyasr opened 2 years ago
These are good ideas. Since all this is internal, this work can be done post-2.0.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
FWIW just tried running the test suite with the normalization removed, and everything passed. I also tried running it throwing an error whenever the normalization changed the input, and the only difference I observed was a conversion from tuples to lists. In terms of performance, the normalization costs a few microseconds for a trivial filter (e.g.{'a': [1]}
) rising to 10s of microseconds for filters including more inputs.
683 replaced the old
Collection
class with a stripped down_SearchIndexer
that does just enough for signac's internal use cases. However, that PR left a few tasks outstanding that we should consider:_SearchIndexer
with a plain dict and free functions. I don't anticipate that these changes will have a significant effect on the code base, but it's worth considering since it may improve clarity. The_SearchIndexer
has essentially no state beyond that of a normal dict, and while its contents are more restricted there is no particular enforcement, so conceptually using a free function might be clearer. Additionally, it is possible to create the object in an invalid state with the current approach. Some additional discussion is here._SearchIndexer
. This is the lowest priority task and would be moot if we reimplemented it as a dict with free functions.