Refactor url methods #264
Merged
Refactor url methods #264
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
vinisalazar
force-pushed
the
refactor-url-methods
branch
from
d82a553
to
972da74
Compare
- 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
force-pushed
the
refactor-url-methods
branch
from
972da74
to
da9ceb1
Compare
|
Added |
vinisalazar
commented
Aug 26, 2022
- 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
force-pushed
the
refactor-url-methods
branch
from
0484fbe
to
04310c7
Compare
- Use Gold Standard ERDDAP server for netCDF tests - Use small 'allDatasets' file
|
Added |
ocefpaf
reviewed
Aug 31, 2022
| @@ -15,7 +15,7 @@ def test__nc_dataset_in_memory_https(): | |||
| """Test loading a netcdf dataset in-memory.""" | |||
| 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 | |||
There was a problem hiding this comment.
That is probably better than a specific dataset_id. Thanks!
ocefpaf
reviewed
Aug 31, 2022
| @@ -59,7 +63,7 @@ def _distinct(url: str, **kwargs: Dict) -> str: | |||
| For example, a query for the variables ["stationType", "stationID"] with `distinct=True` | |||
| 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 | |||
There was a problem hiding this comment.
@MathewBiddle we should probably move this to https at some point.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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_urlandget_categorize_urlfrom theerddapy.erddapymodule to theerddapy.core.urlmodule.For
get_download_url, it's a bit more tricky: it would incur in a circular import betweenerddapy.core.urland the newly-addederddapy.core.griddapmodule, because of theurlopenand_urlopenfunctions. So, I'm thinking whether I should move those functions to a new module (maybeerddapy.core.helpersorerddapy.core.__init__?). Please let me know what you think is best.Thank you,
Vini
Summary of changes (in relation to #263):
get_search_url,get_info_url, andget_categorize_urlto the URL module_search_urlfunction as public