You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
#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:
Replacing the _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.
Remove the potentially unnecessary JSON normalization as discussed in Replace Collection with lightweight _SearchIndexer. #667 (comment) (currently found here). I believe that this function is only invoked in places that have already performed suitable normalization, but we should verify this before making any final changes.
Renaming the _SearchIndexer. This is the lowest priority task and would be moot if we reimplemented it as a dict with free functions.
The text was updated successfully, but these errors were encountered:
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.The text was updated successfully, but these errors were encountered: