Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

BidsComponent.expand() fails to preserve order #346

Closed
pvandyken opened this issue Dec 2, 2023 · 0 comments · Fixed by #348
Closed

BidsComponent.expand() fails to preserve order #346

pvandyken opened this issue Dec 2, 2023 · 0 comments · Fixed by #348
Labels
bug Something isn't working

Comments

@pvandyken
Copy link
Contributor

The tabular format of the BidsComponent zip_list implies an inherent ordering of component entries. Even though snakebids doesn't guarantee any particular insertion order when creating the table, the order should be stable across the lifespan of the component and across any modifications (especially if we support a sorting api in the future).

BidsComponent.expand() currently uses set() to deduplicate the expanded complement of paths, which randomizes the order. A new deduplication method that does not destroy insertion order should be implemented.
@pvandyken pvandyken added the bug Something isn't working label Dec 2, 2023
pvandyken added a commit to pvandyken/snakebids that referenced this issue Dec 10, 2023
@pvandyken
Preserve order of entries in expand method
737905c 
.expand() in all three Expandables was using `set()` to deduplicate
paths following expansion. This was altering the order of the paths,
preventing predicatable expansion

This fix uses `dict.fromkeys` to deduplicate while preserving order

Resolves khanlab#346
@pvandyken pvandyken linked a pull request Dec 10, 2023 that will close this issue
Preserve order of entries in expand method #348
Merged
pvandyken added a commit that referenced this issue Dec 11, 2023
@pvandyken
Preserve order of entries in expand method
dd2ce68 
.expand() in all three Expandables was using `set()` to deduplicate
paths following expansion. This was altering the order of the paths,
preventing predicatable expansion

This fix uses `dict.fromkeys` to deduplicate while preserving order

Resolves #346
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Preserve order of entries in expand method pvandyken/snakebids
1 participant
@pvandyken