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 LoS TEC tag to the GNSS TEC Instrument #96

Merged
merged 33 commits into from
Dec 19, 2023
Merged

Added LoS TEC tag to the GNSS TEC Instrument #96

merged 33 commits into from
Dec 19, 2023

Conversation

aburrell
Copy link
Member

@aburrell aburrell commented Nov 20, 2023

Description

Addresses #12 by adding the 'los' tag to the TEC instrument. Also refactored the general load function to facilitate the new targeted load function needed for the LoS TEC.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality or documentation)
  • This change requires a documentation update

How Has This Been Tested?

import datetime as dt
import pysat
import pysatMadrigal as pymad

stec = pysat.Instrument(inst_module=pymad.instruments.gnss_tec, tag='stec')
stec.download(dt.datetime(2023, 1, 1), user="your name", password='your email')

# Determine which sites are available to load
sites = pymad.instruments.methods.gnss.get_los_receiver_sites(stec.files.files[0])

# Load the last site
stec.load(2023, 1, los_method='site', los_value=sites[-1])

print(stec.variables)

yeilds:

['time', 'gps_site', 'sat_id', 'kindat', 'kinst', 'pierce_alt', 'los_tec', 'dlos_tec', 'tec', 'azm', 'elm', 'gdlat', 'glon', 'rec_bias', 'drec_bias', 'gdlatr', 'gdlonr', 'year', 'month', 'day', 'hour', 'min', 'sec', 'ut1_unix', 'ut2_unix', 'recno']

Test Configuration

  • Operating system: OS X Big Sur
  • Version number: Python 3.9
  • Any details about your local setup that are relevant: develop branch of pysat

Checklist:

  • Make sure you are merging into the develop (not main) branch
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have linted the files updated in this pull request
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • Add a note to CHANGELOG.md, summarizing the changes

If this is a release PR, replace the first item of the above checklist with the
release checklist on the pysat wiki:
https://github.com/pysat/pysat/wiki/Checklist-for-Release

Extracted useful elements of the `load` function to allow their use elsewhere without code duplication.
Added type-specific GNSS load functions and exploratory file functions for the line-of-sight files.
Added support for the line-of-sight (los) TEC data.
Improved the GNSS instrument method description to encompass the new scope.
Added a summary of the changes to the changelog.
Removed excess lines, fixed typos, added missing imports.
Added missing return value to the docstring.
@aburrell
Copy link
Member Author

pysatMadrigal.instruments.methods.general.load complexity reduced from 43 to 27! 🦦

@aburrell aburrell added this to the 0.2.0 Release milestone Nov 20, 2023
@aburrell aburrell added enhancement New feature or request new instrument labels Nov 20, 2023
@aburrell aburrell linked an issue Nov 20, 2023 that may be closed by this pull request
Added two load options for the GNSS TEC LoS tests.
Allow creation of an empty Dataset from an empty DataFrame.
Allow an empty filelist to be provided and create an empty data object.
Allow the GNSS TEC load to work with an empty file list.
Added load tests for the LoS logger warning and ValueError.
Removed unused variable and fixed indentation.
@aburrell
Copy link
Member Author

@jklenzing I am trying to turn off download testing for the new TEC tag, since I don't ever expect the download to work on GA. It looks like I didn't do it right?

aburrell and others added 6 commits November 21, 2023 20:09
Corrected the download test flag.

Co-authored-by: Jeff Klenzing <19592220+jklenzing@users.noreply.github.com>
Fixed the clean method to have an informative message for the LoS data.  Also removed inaccessable line in the load method.
Added a test for the gnss_tec load method ValueError raised if a bad `los_value` is supplied.
Added unit tests for uncovered lines in the general instruments methods sub-module.  Also improved style for Error tests.
Added a test for loading no data in the `load_los` function, additionally testing the time selection set-up.
Added a coveralls service job environment variable.
Needed to get the right dict from the setup.
Copy link
Member

@jklenzing jklenzing left a comment

Choose a reason for hiding this comment

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

The download routine does not work locally for me when running pytest.

Full error message below:

self = <pysatMadrigal.tests.test_instruments.TestInstruments object at 0x123beed90>
inst_dict = {'inst_id': '', 'inst_module': <module 'pysatMadrigal.instruments.gnss_tec' from '/Users/jklenzin/code/core/pysatMadri...ruments/gnss_tec.py'>, 'tag': 'los', 'user_info': {'password': 'pysat.developers@gmail.com', 'user': 'pysat+CI_tests'}}

    @pytest.mark.first
    @pytest.mark.download
    def test_download(self, inst_dict):
        """Test that instruments are downloadable.
    
        Parameters
        ----------
        inst_dict : dict
            Dictionary containing info to instantiate a specific instrument.
            Set automatically from instruments['download'] when
            `initialize_test_package` is run.
    
        """
    
        test_inst, date = initialize_test_inst_and_date(inst_dict)
    
        # Check for username.
        if 'user_info' in inst_dict.keys():
            dl_dict = inst_dict['user_info']
        else:
            dl_dict = {}
        # Note this will download two consecutive days
>       test_inst.download(date, **dl_dict)

../pysat/pysat/tests/classes/cls_instrument_library.py:377: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../pysat/pysat/_instrument.py:3757: in download
    self._download_rtn(date_array, **kwargs)
pysatMadrigal/instruments/gnss_tec.py:247: in download
    general.download(date_array, inst_code=str(madrigal_inst_code),
pysatMadrigal/instruments/methods/general.py:1011: in download
    web_data.downloadFile(mad_file.name, local_file, user, password,
/opt/miniconda3/lib/python3.9/site-packages/madrigalWeb/madrigalWeb.py:1771: in downloadFile
    urlFile = urllib2.urlopen(url, timeout=TIMEOUT)
/opt/miniconda3/lib/python3.9/urllib/request.py:214: in urlopen
    return opener.open(url, data, timeout)
/opt/miniconda3/lib/python3.9/urllib/request.py:523: in open
    response = meth(req, response)
/opt/miniconda3/lib/python3.9/urllib/request.py:632: in http_response
    response = self.parent.error(
/opt/miniconda3/lib/python3.9/urllib/request.py:561: in error
    return self._call_chain(*args)
/opt/miniconda3/lib/python3.9/urllib/request.py:494: in _call_chain
    result = func(*args)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <urllib.request.HTTPDefaultErrorHandler object at 0x123e40850>, req = <urllib.request.Request object at 0x138596580>
fp = <http.client.HTTPResponse object at 0x123e3b6d0>, code = 500, msg = 'Internal Server Error'
hdrs = <http.client.HTTPMessage object at 0x1385af5e0>

    def http_error_default(self, req, fp, code, msg, hdrs):
>       raise HTTPError(req.full_url, code, msg, hdrs, fp)
E       urllib.error.HTTPError: HTTP Error 500: Internal Server Error

/opt/miniconda3/lib/python3.9/urllib/request.py:641: HTTPError

@aburrell
Copy link
Member Author

The download routine does not work locally for me when running pytest.

That's a MadrigalWeb error, nothing we can do about it.

Add a fake file to test the load failures.
Fixed import order to be alphabetical.
Updated coveralls to run in parallel.
Disabled local LoS download tests, also added a warning for users attempting downloads.
Copy link
Member

@jklenzing jklenzing left a comment

Choose a reason for hiding this comment

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

I still have questions about the download routine. This does not work locally or on CI. Does the user download only part of a day of data (thus, breaking the assumptions for the unit tests)?

pysatMadrigal/instruments/gnss_tec.py Show resolved Hide resolved
Removed old test line.
@aburrell
Copy link
Member Author

Does the user download only part of a day of data (thus, breaking the assumptions for the unit tests)?

If MadrigalWeb can't handle the download, then the user has to go and get the files themselves. Not great. Might be worth poking Bill Rideout on this issue.

Updated the version caps for coveralls, sphinx, and sphinx_rtd_theme.
Removed a test variable that was being used for an incorrect purpose.
Removed extra 'run' from workflow.
Updated the coveralls line in the RC CI test.
Added the 'url' kwarg to the DMSP IVM download method, since UTD downloads were failing at the central site.
Removed the URL kwarg, local tests didn't need the url change.
Added a missing index for the test list.
@aburrell aburrell merged commit 819cbb2 into develop Dec 19, 2023
21 of 23 checks passed
@aburrell aburrell deleted the los_tec branch December 19, 2023 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request new instrument
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ENH: add line-of-sight TEC support to gnss_tec instrument
2 participants