Skip to content

Use a list in drop_coords instead of a set for older versions of Scipp#316

Merged
nvaytet merged 2 commits intomainfrom
convert-set-to-list-to-fix-lower-bounds-nightly
Feb 13, 2026
Merged

Use a list in drop_coords instead of a set for older versions of Scipp#316
nvaytet merged 2 commits intomainfrom
convert-set-to-list-to-fix-lower-bounds-nightly

Conversation

@nvaytet
Copy link
Member

@nvaytet nvaytet commented Feb 12, 2026

Older versions of scipp require a list and do not work with a set in drop_coords.
This is to fix the nightly test at lower bounds.

The alternative to this fix would be to bump the min version of scipp in the runtime dependencies of essreduce...

@MridulS
Copy link
Member

MridulS commented Feb 12, 2026

The alternative to this fix would be to bump the min version of scipp in the runtime dependencies of essreduce...

In my opinion being aggresive in dropping old version is better as we currently have the luxury of not having too many downstream users 😅 and we don't need to maintain these quirks.

@nvaytet
Copy link
Member Author

nvaytet commented Feb 12, 2026

In my opinion being aggresive in dropping old version is better as we currently have the luxury of not having too many downstream users 😅 and we don't need to maintain these quirks.

Could be, but in this case, it was such a small change in a single place, that I thought it wasn't worth the time trying to dig through all the releases to find the one with the change. Also, it's not that essreduce can't work at all with the older scipp, so I think being lenient with requirements is worth it.

@nvaytet nvaytet merged commit f729995 into main Feb 13, 2026
4 checks passed
@nvaytet nvaytet deleted the convert-set-to-list-to-fix-lower-bounds-nightly branch February 13, 2026 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants