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

Added NOAA GOES parser #173

Closed
wants to merge 21 commits into from
Closed

Added NOAA GOES parser #173

wants to merge 21 commits into from

Conversation

flynnpc
Copy link
Contributor

@flynnpc flynnpc commented Jun 3, 2019

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

@emiliom emiliom self-requested a review June 3, 2019 20:00
@flynnpc
Copy link
Contributor Author

flynnpc commented Jun 3, 2019

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
Copy link
Contributor

emiliom commented Jun 4, 2019

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

@emiliom
Copy link
Contributor

emiliom commented Jun 26, 2019

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
Copy link
Contributor

emiliom commented Jul 1, 2019

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
Copy link
Contributor

emiliom commented Jul 1, 2019

@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
Copy link
Contributor

emiliom commented Jul 1, 2019

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
Copy link
Contributor Author

flynnpc commented Jul 1, 2019 via email

@emiliom
Copy link
Contributor

emiliom commented Jul 3, 2019

@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
Copy link
Contributor Author

flynnpc commented Jul 3, 2019 via email

@emiliom
Copy link
Contributor

emiliom commented Jul 3, 2019

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

Great! That's really helpful to know.

@emiliom
Copy link
Contributor

emiliom commented Jan 12, 2020

@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.

@emiliom emiliom mentioned this pull request Jan 13, 2020
@solomon-negusse
Copy link
Member

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
Copy link
Contributor

emiliom commented Jan 13, 2020

@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
Copy link
Contributor

emiliom commented Mar 11, 2021

#199 replaces this one. Closing

@emiliom emiliom closed this Mar 11, 2021
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.

3 participants