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

Fermi client fix #2983

Merged
merged 34 commits into from Mar 21, 2019

Conversation

Projects
None yet
4 participants
@hayesla
Copy link
Contributor

commented Mar 12, 2019

Description

This now includes the Fermi data client with new attrs which include Detector and Datatype which can be specified by user

In [1]: from sunpy.net import Fido, attrs as a                                                                        

In [2]: res = Fido.search(a.Time('2015-06-21 00:00', '2015-06-23 23:59'), a.Instrument('gbm'), a.gbm.Detector('n3'), a.gbm.Datatype('ctime'))                                                                                                                  

In [3]: res                                                                                                                                                                                                                                                    
Out[3]: 
<sunpy.net.fido_factory.UnifiedResponse object at 0x11b97ec50>
Results from 1 Provider:

3 Results from the GBMClient:
     Start Time           End Time      Source Instrument Wavelength
       str19               str19         str5     str3       str3   
------------------- ------------------- ------ ---------- ----------
2015-06-21 00:00:00 2015-06-23 23:59:00  FERMI        GBM        nan
2015-06-21 00:00:00 2015-06-23 23:59:00  FERMI        GBM        nan
2015-06-21 00:00:00 2015-06-23 23:59:00  FERMI        GBM        nan

Fixes #2977
2977 No Fermi GBM Client for data download

@nabobalis nabobalis added this to the 1.0 milestone Mar 12, 2019

@sunpy-bot

This comment has been minimized.

Copy link

commented Mar 12, 2019

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

@pep8speaks

This comment has been minimized.

Copy link

commented Mar 12, 2019

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

Line 20:27: E128 continuation line under-indented for visual indent
Line 21:27: E128 continuation line under-indented for visual indent
Line 22:27: E128 continuation line under-indented for visual indent
Line 25:27: E128 continuation line under-indented for visual indent
Line 26:27: E128 continuation line under-indented for visual indent
Line 27:27: E128 continuation line under-indented for visual indent

Comment last updated at 2019-03-21 13:17:55 UTC
@Cadair
Copy link
Member

left a comment

This is great @hayesla thanks a lot!

There are a couple of comments inline, the main one being me checking we can't re-use existing attrs. I have been running on a policy of moving attrs out of their source specific namespace when there is reason to share them between more than one source. So if you need VSO attrs the thing we will do is move them up into sunpy.net.attrs.

Show resolved Hide resolved sunpy/net/dataretriever/attrs/gbm.py Outdated
Show resolved Hide resolved sunpy/net/dataretriever/attrs/gbm.py Outdated
Show resolved Hide resolved sunpy/net/dataretriever/sources/fermi_gbm.py Outdated
Show resolved Hide resolved sunpy/net/attrs.py Outdated
Show resolved Hide resolved sunpy/net/attrs.py Outdated
@hayesla

This comment has been minimized.

Copy link
Contributor Author

commented Mar 14, 2019

hey @Cadair I think this is ready to be approved 👍 :)

@Cadair
Copy link
Member

left a comment

This is looking really great @hayesla!

Would you be able to write some tests as well?

Show resolved Hide resolved sunpy/net/dataretriever/sources/fermi_gbm.py Outdated
Show resolved Hide resolved sunpy/net/dataretriever/sources/fermi_gbm.py
@Cadair

This comment has been minimized.

Copy link
Member

commented Mar 19, 2019

The doc build is failing with:

fermi_gbm.py:docstring of sunpy.net.dataretriever.GBMClient:17:Definition list ends without a blank line; unexpected unindent.

@hayesla hayesla force-pushed the hayesla:fermi_client_fix branch 2 times, most recently from 05561ad to d50462e Mar 20, 2019

@hayesla

This comment has been minimized.

Copy link
Contributor Author

commented Mar 20, 2019

@Cadair I cant figure out how to fix the doc build error - what exactly is wrong with 'fermi_gbm.py:docstring of sunpy.net.dataretriever.GBMClient:17:Definition list ends without a blank line; unexpected unindent.' I've tried to change the indent and where line breaks but it doesn't seem to help .- am I missing something?

@nabobalis

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

So I think the issue (as in I have not tried this myself) with:

    The GBM data comes in daily version files in two formats:
        CSPEC - counts accumulated every  4.096 seconds in 128 energy channels
    for each detector.
        CTIME - counts accumulated every 0.256 seconds in 8 energy channels
    Both of which can be accessed through the attrs 'a.Resolution'.

will be fixed if you try something like:

    The GBM data comes in daily version files in two formats:

        CSPEC - counts accumulated every  4.096 seconds in 128 energy channels for each detector.
        CTIME - counts accumulated every 0.256 seconds in 8 energy channels

    Both of which can be accessed through the attrs 'a.Resolution'.

I am not 100% sure if the first line break is needed. Also whether add ing a* in front of each subpoint in the list is needed.

@Cadair

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

@hayesla you should be able to test the doc build locally by doing:

$ pip/conda install tox
$ tox -e build_docs

which should give you the same output as the CI services.

@hayesla

This comment has been minimized.

Copy link
Contributor Author

commented Mar 20, 2019

ok thanks!

@nabobalis

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

The docs failed due to an example.
sunpy.io.file_tools.UnrecognizedFileTypeError: The requested filetype is not currently supported by SunPy.

@hayesla

This comment has been minimized.

Copy link
Contributor Author

commented Mar 20, 2019

yeah I'm finding it hard to figure out what that error actually is?

@nabobalis

This comment has been minimized.

Copy link
Contributor

commented Mar 20, 2019

Oh that error is not related to your changes. I should have made that clear. Sometimes the download for one example fails and errors. The fail seems to come back every so often.

@Cadair

Cadair approved these changes Mar 21, 2019

Copy link
Member

left a comment

Thanks @hayesla this looks awesome 😁

Show resolved Hide resolved changelog/2983.bugfix.rst Outdated

@nabobalis nabobalis force-pushed the hayesla:fermi_client_fix branch from 5f01d34 to 9d9d44a Mar 21, 2019

@nabobalis

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2019

I just rebased this and tidied up some docstrings. This should be good to go now.

@nabobalis nabobalis merged commit 9698e8a into sunpy:master Mar 21, 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 88.63% of diff hit (target 85.92%)
Details
codecov/project 85.93% (+<.01%) compared to b7bcd07
Details
giles Click details to preview the documentation build
Details
sunpy-bot All checks passed
sunpy.sunpy Build #20190321.12 succeeded
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
@nabobalis

This comment has been minimized.

Copy link
Contributor

commented Mar 21, 2019

Thanks @hayesla!

@hayesla

This comment has been minimized.

Copy link
Contributor Author

commented Mar 21, 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.