Skip to content

Conversation

@vinisalazar
Copy link
Contributor

Hi,

this is a downstream branch of #263, so it will be easier to review after merging that and #259. It includes two additional commits that move get_search_url, get_info_url and get_categorize_url from the erddapy.erddapy module to the erddapy.core.url module.

For get_download_url, it's a bit more tricky: it would incur in a circular import between erddapy.core.url and the newly-added erddapy.core.griddap module, because of the urlopen and _urlopen functions. So, I'm thinking whether I should move those functions to a new module (maybe erddapy.core.helpers or erddapy.core.__init__?). Please let me know what you think is best.

Thank you,
Vini

Summary of changes (in relation to #263):

  • Move the get_search_url, get_info_url, and get_categorize_url to the URL module
  • Refactor the private _search_url function as public
  • Edit docstrings to use gold standard servers

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@vinisalazar vinisalazar force-pushed the refactor-url-methods branch from d82a553 to 972da74 Compare August 17, 2022 13:59
  - Rename from '_search_url' to 'get_search_url'
  - Refactor imports
  - Add docstring
  - Move functions to erddapy.core.url
  - Use functions in ERDDAP class
  - Refactor docstrings to use gold standard servers
@vinisalazar vinisalazar force-pushed the refactor-url-methods branch from 972da74 to da9ceb1 Compare August 17, 2022 20:24
@vinisalazar
Copy link
Contributor Author

Added 0484fbe which refactors the get_download_url method.

  - Refactor method as a function in erddap.core.url
  - Use standalone function in ERDDAP class
  - Refactor imports accordingly (in tests as well)
    - Add '__all__' variable to erddapy
@vinisalazar vinisalazar force-pushed the refactor-url-methods branch from 0484fbe to 04310c7 Compare August 29, 2022 20:37
  - Use Gold Standard ERDDAP server for netCDF tests
  - Use small 'allDatasets' file
@vinisalazar
Copy link
Contributor Author

Added 66545d1 in an effort to prevent tests failing due to server problems, but not sure if a specific formatting was needed for that file.

from netCDF4 import Dataset

url = "https://podaac-opendap.jpl.nasa.gov/opendap/allData/modis/L3/aqua/11um/v2019.0/4km/daily/2017/365/AQUA_MODIS.20171231.L3m.DAY.NSST.sst.4km.nc" # noqa
url = "http://erddap.ioos.us/erddap/tabledap/allDatasets.nc" # noqa
Copy link
Member

Choose a reason for hiding this comment

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

That is probably better than a specific dataset_id. Thanks!

will return a sorted list of "stationIDs" associated with each "stationType".
See https://coastwatch.pfeg.noaa.gov/erddap/tabledap/documentation.html#distinct
See http://erddap.ioos.us/erddap/tabledap/documentation.html#distinct
Copy link
Member

Choose a reason for hiding this comment

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

@MathewBiddle we should probably move this to https at some point.

@ocefpaf ocefpaf merged commit 29f6e69 into ioos:main Aug 31, 2022
@ocefpaf ocefpaf added the GSoC22 label Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants