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

Replace SunPy's Downloader with an asyncio version #2797

Merged
merged 34 commits into from Mar 16, 2019

Conversation

Projects
5 participants
@Cadair
Copy link
Member

commented Oct 16, 2018

This makes use of https://github.com/Cadair/parfive which I implemented independently of SunPy as it is rather more generic than just SunPy.

TODO:

  • Handle overwriting / skipping downloads correctly.
  • Add an example or something dealing with .errors.
  • Package parfive on conda. (In progress here)
  • Remove custom overwrite handling in JSOCClient.
  • Documentation

To those reviewing this, it would also be worthwhile reviewing the actual downloader code which is in a reviewable format here: https://github.com/Cadair/parfive/pull/5/files

This is how it looks in a notebook:
peek 2018-12-20 14-16

and this is how it looks in a terminal:
peek 2018-12-20 14-22

test script: https://gist.github.com/6dafef3cfe47e7d6659ab8df44475126

Fixes #555
Fixes #2376
Fixes #2773
Fixes #2108
Fixes #1331
Fixes #1280

@sunpy-bot

This comment has been minimized.

Copy link

commented Oct 16, 2018

Thanks for the pull request @Cadair! Everything looks great!

@pep8speaks

This comment was marked as outdated.

Copy link

commented Oct 16, 2018

Hello @Cadair! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 224:5: E129 visually indented line with same indent as next logical line

Line 344:101: E501 line too long (111 > 100 characters)

Comment last updated at 2019-03-16 12:12:48 UTC

@Cadair Cadair added this to the 1.0 milestone Oct 16, 2018

@Cadair Cadair changed the title Replace SunPys Downloader with an asyncio version Replace SunPy's Downloader with an asyncio version Oct 24, 2018

@dpshelio
Copy link
Member

left a comment

I prefer to keep the dependency as external. We could benefit more of having it in that way and other people could use it

.travis.yml Outdated
@@ -36,7 +36,7 @@ env:
- SETUP_CMD='test --coverage'
- CONDA_CHANNELS='sunpy'
- CONDA_DEPENDENCIES='openjpeg Cython jinja2 scipy matplotlib pytest==3.6 mock requests beautifulsoup4 sqlalchemy scikit-image pytest-mock lxml pyyaml pandas pytest-astropy suds-jurko glymur pytest-xdist dask drms sphinx-astropy pytest-cov hypothesis'
- PIP_DEPENDENCIES='pytest-rerunfailures sunpy-sphinx-theme pytest-sugar'
- PIP_DEPENDENCIES='pytest-rerunfailures sunpy-sphinx-theme pytest-sugar git+https://github.com/Cadair/parfive'

This comment has been minimized.

Copy link
@dpshelio

dpshelio Nov 10, 2018

Member

You have it in pip already, shouldn't just depend from there?

This comment has been minimized.

Copy link
@Cadair

Cadair Dec 19, 2018

Author Member

eventually, but at the moment I just need to keep it upto date with master.

@Cadair Cadair added this to High Priority Features in SunPy 1.0 Dec 13, 2018

@Cadair Cadair moved this from High Priority Features to Release Blocking Items in SunPy 1.0 Dec 14, 2018

@Cadair Cadair moved this from Release Blocking Items to High Priority Features in SunPy 1.0 Dec 14, 2018

@Cadair Cadair force-pushed the Cadair:parfive branch 2 times, most recently from cac5f81 to 2b20d27 Dec 19, 2018

@dpshelio

This comment has been minimized.

Copy link
Member

commented Dec 22, 2018

screenshot_20181222-143143

It looks nice also on termux, but in landscape.

@Cadair

This comment has been minimized.

Copy link
Member Author

commented Feb 28, 2019

I have updated parfive to support flexible overwrite behaviour, so I will now update this to match.

@Cadair Cadair force-pushed the Cadair:parfive branch 2 times, most recently from 9d3af7b to 2a98566 Mar 2, 2019

@Cadair Cadair moved this from High Priority Features to Release Blocking Items in SunPy 1.0 Mar 6, 2019

@Cadair Cadair force-pushed the Cadair:parfive branch from bfc79e3 to 8494142 Mar 12, 2019

@Cadair

This comment has been minimized.

Copy link
Member Author

commented Mar 12, 2019

OK, barring documentation I think this is now feature complete. I will do a release of parfive and then move onto the docs.

@hayesla

This comment has been minimized.

Copy link
Contributor

commented Mar 12, 2019

@Cadair I've been testing this during the morning - love that it can handle overwrite! 👐

Show resolved Hide resolved .circleci/config.yml Outdated
Show resolved Hide resolved sunpy/net/base_client.py Outdated
Show resolved Hide resolved sunpy/net/base_client.py Outdated
Show resolved Hide resolved sunpy/net/base_client.py Outdated
Show resolved Hide resolved sunpy/net/base_client.py Outdated
Show resolved Hide resolved sunpy/net/base_client.py Outdated
Show resolved Hide resolved sunpy/net/base_client.py Outdated

Cadair and others added some commits Mar 12, 2019

Apply suggestions from code review
Co-Authored-By: Cadair <stuart@cadair.com>

@Cadair Cadair force-pushed the Cadair:parfive branch from 38888b6 to 614d9c1 Mar 15, 2019

Show resolved Hide resolved docs/guide/acquiring_data/fido.rst Outdated
Show resolved Hide resolved sunpy/net/tests/test_fido.py

@Cadair Cadair force-pushed the Cadair:parfive branch from 5c4dabc to b271d89 Mar 15, 2019

Cadair added some commits Mar 15, 2019

@nabobalis nabobalis merged commit a557fc1 into sunpy:master Mar 16, 2019

16 checks passed

ci/circleci: 32bit Your tests passed on CircleCI!
Details
ci/circleci: egg-info-36 Your tests passed on CircleCI!
Details
ci/circleci: egg-info-37 Your tests passed on CircleCI!
Details
ci/circleci: figure-tests-36 Your tests passed on CircleCI!
Details
ci/circleci: html-docs Your tests passed on CircleCI!
Details
ci/circleci: pip-install Your tests passed on CircleCI!
Details
codecov/patch 93.1% of diff hit (target 85.51%)
Details
codecov/project 85.8% (+0.29%) compared to 0db0b52
Details
giles Click details to preview the documentation build
Details
sunpy-bot All checks passed
sunpy.sunpy Build #20190316.3 has test failures
Details
sunpy.sunpy (Linux_36_Conda_offline) Linux_36_Conda_offline succeeded
Details
sunpy.sunpy (Linux_36_offline) Linux_36_offline succeeded
Details
sunpy.sunpy (Linux_37_online) Linux_37_online succeeded
Details
sunpy.sunpy (Windows_36_offline) Windows_36_offline succeeded
Details
sunpy.sunpy (macOS_37_offline) macOS_37_offline succeeded
Details
@Cadair

This comment has been minimized.

Copy link
Member Author

commented Mar 16, 2019

omfg whooottt!

@Cadair Cadair deleted the Cadair:parfive branch Mar 16, 2019

@Cadair Cadair moved this from Release Blocking Items to Finished in SunPy 1.0 Mar 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.