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

Implement a simple atlas manager widget #153

Merged
merged 6 commits into from
Jul 25, 2024

Conversation

alessandrofelder
Copy link
Member

@alessandrofelder alessandrofelder commented Jul 24, 2024

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?
We want to separate out atlas download and version management from visualisation.

What does this PR do?
brainrender-napari now provides two widgets, one for atlas management, one for atlas visualisation.

References

Will close #148
This is also a step on the way to #128 and #129
I think this is the best quickfix to get us to a better state than now (with docs and released version out-of-sync), so we can focus on other things as core-devs for a while (after some changes to the docs).

How has this PR been tested?

Integration test added. Manager widget was already there and tested under the hood, just not there in napari.

Is this a breaking change?

Minor changes in naming.

Does this PR require an update to the documentation?

Yes, to be addressed with brainglobe/brainglobe.github.io#222

Checklist:

Copy link

codecov bot commented Jul 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.06%. Comparing base (e5d6a3b) to head (0deb9b0).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #153      +/-   ##
==========================================
+ Coverage   97.99%   98.06%   +0.06%     
==========================================
  Files          10       11       +1     
  Lines         448      464      +16     
==========================================
+ Hits          439      455      +16     
  Misses          9        9              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alessandrofelder alessandrofelder marked this pull request as ready for review July 24, 2024 17:08
@alessandrofelder alessandrofelder requested a review from a team July 24, 2024 17:10
Copy link
Member

@adamltyson adamltyson left a comment

Choose a reason for hiding this comment

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

LGTM, one tiny suggestion.

I think there's lots of scope to make this easier to use, but I agree that this should just be merged and released, then looked into in the future.

The tests are failing because it took too long to download an atlas. It might be worth giving it a bit more time, GIN can be pretty slow (especially as brainglobe-utils tests are hammering it at the moment).

brainrender_napari/brainrender_manager_widget.py Outdated Show resolved Hide resolved
@adamltyson adamltyson self-requested a review July 25, 2024 12:52
@alessandrofelder alessandrofelder merged commit 763e4b3 into main Jul 25, 2024
13 checks passed
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.

[BUG] Atlas entries do not show on fresh install
2 participants