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

Loading atlas with no Internet #358

Merged
merged 8 commits into from
Aug 23, 2024

Conversation

saarah815
Copy link
Contributor

Before submitting a pull request (PR), please read the contributing guide.

Please fill out as much of this template as you can, but if you have any problems or questions, just leave a comment and we will help out :)

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?

Outputs appropriate error messages when user tries to load atlas with no Internet connection.

What does this PR do?

In bg_atlas.BrainGlobeAtlas:

  • utils.check_internet_connection() checks for Internet connection, and also checks if GIN is down. If there is no Internet connection, code checks to see if the atlas they want is cached and available locally. If so, continues as normal.
  • If atlas unavailable locally, code tells user to connect to the Internet in order to download the atlas.

In list_atlases.get_all_atlases_lastversions():

  • utils.check_internet_connection() checks for Internet connection, and also checks if GIN is down. If there is an Internet connection, get_all_atlases_lastversions() performs as before and returns the updated list of available atlases from last_versions.conf from GIN.
  • If there is no Internet, get_all_atlases_lastversions() warns user that they are offline. If user has a local copy of last_versions.conf in /.brainglobe/, get_all_atlases_lastversions() will call a new function utils.conf_from_file to return the list of available atlases based on this local conf file, but will also warn the user that this list could be outdated and that they should connect to the Internet for an updated list.
  • If the user does not have a local copy of last_versions.conf in /.brainglobe/, get_all_atlases_lastversions() will tell user that they must connect to the Internet to fetch atlas list.

References

Issue #142

How has this PR been tested?

Tested locally with different situations:

In bg_atlas.BrainGlobeAtlas:

  • No Internet but atlas exists locally:

image

  • No Internet and does not exist locally:

image

  • Connected to the Internet and exists locally:

[Works as expected, no output]

  • Connected to the Internet, but does not exist locally:

image

In get_all_atlases_lastversions():

  • No Internet and last_versions.conf does not exist in local /.brainglobe/:

image

  • No Internet and last_versions.conf does exist in local /.brainglobe/ (but could be outdated):

image

(Note: I am explicitly printing the list here)

  • Connected to the Internet:

image

(Works as expected and gets last_versions.conf from GIN with no error messages. Again, I am explicitly printing the list here)

Is this a breaking change?

No.

Does this PR require an update to the documentation?

N/A

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality (unit & integration)
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit

@IgorTatarnikov IgorTatarnikov self-requested a review August 16, 2024 15:42
@IgorTatarnikov
Copy link
Member

This is great! I'll do a bit of refactoring and get this reviewed.

@IgorTatarnikov
Copy link
Member

This should cover everything.

The last_versions.conf file should now be cached to disk (in the brainglobe directory as defined by the config) every time it's fetched from GIN. If the internet/GIN is down, this local cached copy can be used to display the available atlaes.

Errors and warnings should now separate when the internet is not available vs when GIN itself is down.

@IgorTatarnikov IgorTatarnikov requested a review from a team August 23, 2024 12:56
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.

very nice.

@alessandrofelder alessandrofelder merged commit e4d5c6f into brainglobe:main Aug 23, 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.

3 participants