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.

Sign up for GitHub

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

Jump to bottom

Data downloader #959

Merged
merged 62 commits into from
Aug 17, 2023
Merged

Data downloader #959

merged 62 commits into from
Aug 17, 2023

Conversation

j-c-c
Copy link
Collaborator

@j-c-c j-c-c commented Jun 7, 2023 •
edited
Loading

This PR adds download functionality for example datasets.

Closes #712

@j-c-c j-c-c added documentation Improvements or additions to documentation enhancement New feature or request dependencies Pull requests that update a dependency file labels Jun 7, 2023
@j-c-c j-c-c self-assigned this Jun 7, 2023
@codecov
Copy link

codecov bot commented Jun 7, 2023 •
edited
Loading

Codecov Report

Merging #959 (3167065) into develop (2011881) will decrease coverage by 0.37%.
The diff coverage is 42.52%.

@@             Coverage Diff             @@
##           develop     #959      +/-   ##
===========================================
- Coverage    88.97%   88.60%   -0.37%     
===========================================
  Files          123      126       +3     
  Lines        11048    11132      +84     
===========================================
+ Hits          9830     9864      +34     
- Misses        1218     1268      +50
Files Changed Coverage Δ
src/aspire/downloader/data_fetcher.py 32.39% <32.39%> (ø)
src/aspire/volume/volume.py 93.11% <66.66%> (-0.64%) ⬇️
src/aspire/__init__.py 100.00% <100.00%> (ø)
src/aspire/downloader/__init__.py 100.00% <100.00%> (ø)
src/aspire/downloader/registry.py 100.00% <100.00%> (ø)
src/aspire/image/image.py 93.38% <100.00%> (+0.02%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@j-c-c j-c-c force-pushed the data_downloader branch 2 times, most recently from a49c56c to ae129bc Compare June 23, 2023 15:43
@j-c-c j-c-c force-pushed the data_downloader branch 2 times, most recently from f321b49 to c9a5d78 Compare July 19, 2023 16:25
@j-c-c j-c-c requested a review from garrettwrong July 19, 2023 19:25
garrettwrong
garrettwrong requested changes Jul 27, 2023
View reviewed changes
Copy link
Collaborator

@garrettwrong garrettwrong left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together. Mainly some string stuff and a couple questions (which are likely non issues).

If you can get it back to me today/tomorrow I should be able to re-review it quickly. Thanks!

.github/workflows/workflow.yml Outdated Show resolved Hide resolved
.github/workflows/workflow.yml Outdated Show resolved Hide resolved
.github/workflows/workflow.yml Outdated Show resolved Hide resolved
gallery/experiments/simulated_abinitio_pipeline.py Outdated Show resolved Hide resolved
gallery/tutorials/aspire_introduction.py Outdated Show resolved Hide resolved
src/aspire/downloader/data_fetcher.py Outdated Show resolved Hide resolved
src/aspire/downloader/data_fetcher.py Outdated Show resolved Hide resolved
src/aspire/image/image.py Outdated Show resolved Hide resolved
src/aspire/volume/volume.py Show resolved Hide resolved
src/aspire/volume/volume.py Show resolved Hide resolved
@j-c-c j-c-c requested a review from garrettwrong July 28, 2023 15:23
garrettwrong
garrettwrong previously approved these changes Jul 28, 2023
View reviewed changes
Copy link
Collaborator

@garrettwrong garrettwrong left a comment

Choose a reason for hiding this comment

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

Nice, thanks. One question you can answer now (url thing) and another to leave for Joakim's round. Otherwise looking good. Will be nice to get this in.

gallery/tutorials/aspire_introduction.py Show resolved Hide resolved
gallery/tutorials/tutorials/data_downloader.py Show resolved Hide resolved
@j-c-c j-c-c marked this pull request as ready for review July 28, 2023 18:48
@j-c-c j-c-c requested a review from janden as a code owner July 28, 2023 18:48
janden
janden requested changes Aug 1, 2023
View reviewed changes
Copy link
Collaborator

@janden janden left a comment

Choose a reason for hiding this comment

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

Looks nice! Just a couple of things.

src/aspire/config_default.yaml Show resolved Hide resolved
src/aspire/downloader/data_fetcher.py Outdated Show resolved Hide resolved
src/aspire/downloader/data_fetcher.py Outdated Show resolved Hide resolved
src/aspire/downloader/data_fetcher.py Outdated Show resolved Hide resolved
@j-c-c j-c-c requested a review from janden August 2, 2023 15:12
@j-c-c
Use dict to return file/filepath for show_downloads.
57d37c3
@j-c-c
finufft workaround in Volume.load.
dae5f11
@j-c-c
remove show_downloads method.
a19e93a
@j-c-c
add cache to ampere CI
b1bc74d
@j-c-c
remove unused imports
74f105d
@j-c-c
edit function name in init
86cb824
@j-c-c
fix spelling
b18ad53
@j-c-c
remove deleted method from init
a5ed9ae
@j-c-c
fix ampere cache
20ef23d
@j-c-c
remove extra newline.
0e7387b
@j-c-c
Add data_downloader tutorial. Fix rows in image.show().
2c7a025
@j-c-c
fix mistakenly moved utils init imports.
bb750b4
@j-c-c
tox
0d9a1db
@j-c-c
edit tutorial subtitle
f782729
@j-c-c
Add docstring params.
e5938d4
@j-c-c
fetch_data docstring
ab8d022
@j-c-c
CI workflow review updates.
697f76c
@j-c-c
workflow syntax
e29e6d0
@j-c-c
fix workflow... again.
6ac939b
@j-c-c
update gallery to address review.
650da2e
@j-c-c
address data_fetcher review.
a0ed629
@j-c-c
Use traditional ceiling divide.
dfda4ba
@j-c-c
remove unused import
1a3cc0b
@j-c-c
clean_downloads ~~> remove_downloads.
d458089
@j-c-c
typo. Make method private.
a2b01fb
@j-c-c
small docstring change.
3167065
garrettwrong
garrettwrong approved these changes Aug 17, 2023
View reviewed changes
Copy link
Collaborator

@garrettwrong garrettwrong left a comment

Choose a reason for hiding this comment

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

🚀

@j-c-c j-c-c merged commit 5127c94 into develop Aug 17, 2023
32 checks passed
@j-c-c j-c-c mentioned this pull request Aug 29, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garrettwrong garrettwrong garrettwrong approved these changes

@janden janden janden approved these changes

Assignees

@j-c-c j-c-c

Labels
dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@j-c-c @janden @garrettwrong