ulmo-dev / ulmo

clean, simple and fast access to public hydrology and climatology data.
http://ulmo.readthedocs.org
Other
169 stars 63 forks source link

Added NOAA GOES parser #173

Closed flynnpc closed 3 years ago

flynnpc commented 5 years ago

I forgot to add the noaa files to my previous pull request. Sorry about that. This should replace eddn.

flynnpc commented 5 years ago

The eddn website has the message (https://eddn.usgs.gov/) which prohibits further web scraping. I re-wrote the script to get dcp messages from NOAA's field test site, which has granted us permission, instead. Furthermore, eddn was deleted in this version to avoid future use.

emiliom commented 5 years ago

Thanks @flynnpc! I'll try to review this at the end of the week.

emiliom commented 5 years ago

Sorry I haven't been able to get back to this -- and thanks for the new commits! I'll get to it by Monday.

emiliom commented 5 years ago

1 of the 5 new tests for the new NOAA GOES parser is failing: test/noaa_goes_test.py::test_parse_dcp_message_timestamp. See https://travis-ci.org/ulmo-dev/ulmo/jobs/550913422#L1268 and https://travis-ci.org/ulmo-dev/ulmo/jobs/550913422#L2088

Just adding this comment here for reference. I'll look at the test and the errors more closely later. @flynnpc if you can take a look, that'd be great.

And note to self: many other ulmo tests are now failing, whereas they were fine 3 months ago. #174

emiliom commented 5 years ago

@flynnpc The EDDN parser you're replacing had nice header text describing them, here:

I'm not familiar with the GEOS Satellite DCP messages, but my understanding is that what you're doing now from NOAA gets at exactly the same data as the EDDN parser, right? So, hopefully you can reuse most of the header descriptive text from those two modules. Would the replacements for the two EDDN links be https://dcs1.noaa.gov/ and https://dcs1.noaa.gov/LRGS/DCP-Data-Service-14.pdf ? Thanks.

emiliom commented 5 years ago

Once this is merged, and before issuing a new release, we'll want to add an entry for this parser in the ulmo documentation. And remove the EDDN documentation.

flynnpc commented 5 years ago

Hey Emilio. I'll look into the failing tests and updating the documentation too. Thanks.

On Mon, Jul 1, 2019 at 1:24 AM Emilio Mayorga notifications@github.com wrote:

1 of the 5 new tests for the new NOAA GOES parser is failing: test/noaa_goes_test.py::test_parse_dcp_message_timestamp. See https://travis-ci.org/ulmo-dev/ulmo/jobs/550913422#L1268 and https://travis-ci.org/ulmo-dev/ulmo/jobs/550913422#L2088

Just adding this comment here for reference. I'll look at the test and the errors more closely later. @flynnpc https://github.com/flynnpc if you can take a look, that'd be great.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ulmo-dev/ulmo/pull/173?email_source=notifications&email_token=ABNZYY4J2KR3VZ3OGAZBN2DP5GPKVA5CNFSM4HSO7TO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODY5DSCI#issuecomment-507132169, or mute the thread https://github.com/notifications/unsubscribe-auth/ABNZYYYN5KENH2Y7RGNYIOLP5GPKVANCNFSM4HSO7TOQ .

--

Paul FlynnGeography and the Environment University of Texas at Austin

emiliom commented 5 years ago

@flynnpc The test/noaa_goes_test.py::test_parse_dcp_message_timestamp test is still failing (https://travis-ci.org/ulmo-dev/ulmo/jobs/553802790#L1162). I can take a closer look later on and will run it locally, but in the meantime, do you have any insight on that failure? Are you able to run the NOAA GOES parser on your own system w/o problems?

flynnpc commented 5 years ago

I just tried running the test on my machine and it works.

On Wed, Jul 3, 2019 at 11:51 AM Emilio Mayorga notifications@github.com wrote:

@flynnpc https://github.com/flynnpc The test/noaa_goes_test.py::test_parse_dcp_message_timestamp test is still failing (https://travis-ci.org/ulmo-dev/ulmo/jobs/553802790#L1162). I can take a closer look later on and will run it locally, but in the meantime, do you have any insight on that failure? Are you able to run the NOAA GOES parser on your own system w/o problems?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ulmo-dev/ulmo/pull/173?email_source=notifications&email_token=ABNZYYZFHXF4UGS4MEC7XXDP5TKIDA5CNFSM4HSO7TO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZFBQPQ#issuecomment-508172350, or mute the thread https://github.com/notifications/unsubscribe-auth/ABNZYYY3HKFMWY7KY6VKKJ3P5TKIDANCNFSM4HSO7TOQ .

--

Paul FlynnGeography and the Environment University of Texas at Austin

emiliom commented 5 years ago

I just tried running the test on my machine and it works.

Great! That's really helpful to know.

emiliom commented 4 years ago

@flynnpc I'm only now getting back to this. Sorry ...

I finally got around to being able to run the tests locally, but I'm running into the same test failure with test/noaa_goes_test.py::test_parse_dcp_message_timestamp I pointed out earlier on Travis-CI. Back then you reported that you the test passed locally on your machine. But running into the same error on both my local machine and Travis-CI, 6 months apart, suggests something is going on. Could there be a specific configuration you have set up that we're missing?

For reference, here's how I'm running the tests locally, adapted from .travis.yml:

export PYTHON_VERSION="3.6"
export ULMO_ENV=ulmo_dev
export ULMO_CACHE_DIR=./ulmo_test_cache
export COVERAGE=$HOME/miniconda/envs/$ULMO_ENV/lib/python$PYTHON_VERSION/site-packages/ulmo
pytest --cov=$COVERAGE -vv -k 'noaa_goes'

I created the ulmo_env conda environment like this:

conda env create -n ulmo_dev --file conda_environment.yml
python setup.py develop

This conda env and testing approach has been working well for me for running ulmo tests locally.

Thanks for your help and patience! I'm working towards a new ulmo release and it'd be great to include this PR.

solomon-negusse commented 4 years ago

Hi @emiliom, Paul isn't working for TWDB anymore and this issue is in our backlog for my new colleague to finalize it but may not make it for this release.

emiliom commented 4 years ago

@solomon-negusse Thanks for the heads-up and for still allocating time to this. I'll go ahead with the upcoming release without this PR, but can issue another release once the NOAA GOES reader is vetted and fully merged.

emiliom commented 3 years ago

199 replaces this one. Closing