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.

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

Update brainglobe-napari-io import #172

Merged
merged 2 commits into from
Jan 8, 2024
Merged

Update brainglobe-napari-io import #172

merged 2 commits into from
Jan 8, 2024

Conversation

adamltyson
Copy link
Member

@adamltyson adamltyson commented Jan 4, 2024

Requires brainglobe/brainglobe-napari-io#61 to be merged (tests will fail until it's released), and when a new version is released, it will need to pin the latest version of brainglobe-napari-io.

Copy link
Member

@alessandrofelder alessandrofelder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
Given that the new brainglobe-napari-io is released, should we pin the version in this PR (and then merge and release)?

@willGraham01
Copy link
Contributor

willGraham01 commented Jan 8, 2024

@alessandrofelder sorry for the dismissal I was only following your recommendation 😂

But have now pinned the new version in this update. In cases like this where we have tool updates that are dependent on releases, I think it's important we always pin new versions in the pull requests. The meta-package should handle them, but giving it some help wouldn't go amiss :L

@alessandrofelder
Copy link
Member

I think it's important we always pin new versions in the pull requests. The meta-package should handle them ...

The metapackage (at least its user-facing part, not the future benchmarks that will depend on the dev version) will only install released versions, right. So should this technically be

I think it's important we always pin new versions before releasing?

?

@adamltyson
Copy link
Member Author

Pinning (the minimum) version is always probably a good idea though right, so that pip install brainglobe -U will fetch the correct packages.

@willGraham01
Copy link
Contributor

Pinning (the minimum) version is always probably a good idea though right, so that pip install brainglobe -U will fetch the correct packages.

Yeah sorry for the unhelpful comment - this is what I meant 😅 In this case, brainglobe-napari-io>=0.3.2 is now required by brainreg, so we should add that in our update to brainreg.

  • The meta-package will ensure that brainglobe-napari-io and brainreg are both fetched on install.
  • pip will pick up that the latest brainreg version requires brainglobe-napari-io>=0.3.2, and make sure that the latest brainreg version is installed with brainglobe-napari-io>=0.3.2.
  • So, even though the meta-package currently only forces ``brainglobe-napari-io>=0.3.0`, including changes like this (one tool depends on another so update the min version) in the update PR will keep everyone happy

@adamltyson adamltyson merged commit faba4ac into main Jan 8, 2024
14 checks passed
@adamltyson adamltyson deleted the brainglobe-io branch January 8, 2024 16:12
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.

3 participants