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

Automatic data downloading with pooch #796

Merged
merged 15 commits into from
Jun 19, 2024
Merged

Automatic data downloading with pooch #796

merged 15 commits into from
Jun 19, 2024

Conversation

loostrum
Copy link
Member

@loostrum loostrum commented Jun 12, 2024

A downloader utility was added, which contains an overview of all data files we use, hosted on either Github or Zenodo.
With the dianna.utils.downloader.download function, one can download and use any of these files. The files are downloaded to an OS cache directory by default, and the checksums are verified so any changes in the online files are detected.

The tutorials were updated to use this functionality.

While rerunning the notebooks I noticed the colormaps were mostly wrong (default viridis instead of bwr). This is due to #776, where the colormaps were removed from the notebooks, but the defaults were not set in the dianna code for all modalities (only in the dashboard it seems). In this PR I added the bwr default to the visualization module where not already present.

@loostrum loostrum linked an issue Jun 12, 2024 that may be closed by this pull request
15 tasks
@loostrum loostrum added the standup Temp label- for disscussion with the team next standup label Jun 19, 2024
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@loostrum loostrum removed the standup Temp label- for disscussion with the team next standup label Jun 19, 2024
@loostrum loostrum marked this pull request as ready for review June 19, 2024 10:02
Copy link
Contributor

@cwmeijer cwmeijer left a comment

Choose a reason for hiding this comment

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

Looks great! I tried one notebook and run the tests. Awesome! 🍕🍔🍟

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice and thorough! This is quite a heavy test. Could we run this for only one build config for instance instead of all build configurations?

data_cmap=None,
show_plot=True,
output_filename=None):
def plot_image(
Copy link
Contributor

Choose a reason for hiding this comment

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

why this reformatting? just some autoformatter I suppose? it makes the pr a bit larger.

Run downloader test in CI only in single build
@loostrum loostrum marked this pull request as draft June 19, 2024 14:09
@loostrum loostrum marked this pull request as ready for review June 19, 2024 14:16
@loostrum loostrum merged commit 7dbd77b into main Jun 19, 2024
25 of 27 checks passed
@loostrum loostrum deleted the 171-auto-data-download branch June 19, 2024 15:12
@elboyran
Copy link
Contributor

elboyran commented Jun 21, 2024

@loostrum if all models are downloaded from Zenood now, why to keep the models folder? Or at least to remove the non-needed files?

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.

Update tutorial notebooks with direct data and model downloads (when possible)
3 participants