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.

Sign up for GitHub

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

update the dependency and temporal filter for python_cmr 0.10.0 #520

Closed
wants to merge 11 commits into from
+699 −612
Closed

update the dependency and temporal filter for python_cmr 0.10.0 #520

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
979c982
directly use python_cmr's temporal
itcarroll Apr 12, 2024
50b46d3
add lxml-html-clean
itcarroll Apr 12, 2024
463c2f3
better documentaion
itcarroll Apr 15, 2024
998bcf0
bump python-cmr
itcarroll Apr 15, 2024
4dccdf7
deps change with python-cmr update
itcarroll Apr 15, 2024
e4ed258
syntax
itcarroll Apr 15, 2024
5dc411a
lint
itcarroll Apr 15, 2024
2d7a8d2
remove conda-forge workaround
itcarroll Apr 16, 2024
a9a4801
Merge branch 'main' into temporal
itcarroll Apr 19, 2024
c7a311a
poetry lock without updates
itcarroll Apr 19, 2024
977231b
test utc offset
itcarroll Apr 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions ci/environment-mindeps.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,12 @@ channels:
dependencies:
# required dependencies
- python=3.8
- python-cmr=0.9.0
- pqdm=0.1
- requests=2.26
- s3fs=2022.11
- fsspec=2022.11
- tinynetrc=1.3.1
- multimethod=1.8
- python-dateutil=2.8.2
- importlib-resources=6.3.2
# test dependencies
- responses
Expand All @@ -26,3 +24,6 @@ dependencies:
- types-setuptools
- ruff
- vcrpy
- pip
- pip:
- python-cmr==0.10.0
itcarroll marked this conversation as resolved.
Show resolved Hide resolved
20 changes: 9 additions & 11 deletions docs/tutorials/SSL.ipynb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,10 +102,8 @@
"source": [
"granules = earthaccess.search_data(\n",
" short_name = \"SEA_SURFACE_HEIGHT_ALT_GRIDS_L4_2SATS_5DAY_6THDEG_V_JPL2205\",\n",
" temporal = (\"2017-01\",\"2018-01\"),\n",
" count = -1\n",
")\n",
"print(len(granules))"
" count = 1\n",
")"
]
},
{
Expand Down Expand Up @@ -185,10 +183,10 @@
" print(f\"Retrieving data for year: {year}\")\n",
" results = earthaccess.search_data(\n",
" doi = \"10.5067/SLREF-CDRV3\",\n",
" temporal=(f\"{year}-05\", f\"{year}-06\")\n",
" temporal=(f\"{year}-05\", f\"{year}-05\"),\n",
" count = 1,\n",
" )\n",
" if len(results)>0:\n",
" granules.append(results[0])\n",
" granules.append(results[0])\n",
"print(f\"Total granules: {len(granules)}\")"
]
},
Expand Down Expand Up @@ -259,11 +257,11 @@
"def ssl_area(lats):\n",
" \"\"\"\n",
" Calculate the area associated with a 1/6 by 1/6 degree box at latitude specified in 'lats'.\n",
" \n",
"\n",
" Parameter\n",
" ==========\n",
" lats: a list or numpy array of size N the latitudes of interest. \n",
" \n",
" lats: a list or numpy array of size N the latitudes of interest.\n",
"\n",
" Return\n",
" =======\n",
" out: Array (N) area values (unit: m^2)\n",
Expand Down Expand Up @@ -357,7 +355,7 @@
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.9.16"
"version": "3.11.8"
}
},
"nbformat": 4,
Expand Down
4 changes: 2 additions & 2 deletions earthaccess/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def search_datasets(
* **daac**: e.g. NSIDC or PODAAC
* **provider**: particular to each DAAC, e.g. POCLOUD, LPDAAC etc.
* **temporal**: a tuple representing temporal bounds in the form
`("yyyy-mm-dd", "yyyy-mm-dd")`
`(date_from, date_to)`
* **bounding_box**: a tuple representing spatial bounds in the form
`(lower_left_lon, lower_left_lat, upper_right_lon, upper_right_lat)`

Expand Down Expand Up @@ -96,7 +96,7 @@ def search_data(
* **daac**: e.g. NSIDC or PODAAC
* **provider**: particular to each DAAC, e.g. POCLOUD, LPDAAC etc.
* **temporal**: a tuple representing temporal bounds in the form
`("yyyy-mm-dd", "yyyy-mm-dd")`
`(date_from, date_to)`
* **bounding_box**: a tuple representing spatial bounds in the form
`(lower_left_lon, lower_left_lat, upper_right_lon, upper_right_lat)`

Expand Down
81 changes: 30 additions & 51 deletions earthaccess/search.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
from inspect import getmembers, ismethod
from typing import Any, List, Optional, Tuple, Type, Union

import dateutil.parser as parser # type: ignore
import requests
from cmr import CollectionQuery, GranuleQuery

Expand Down Expand Up @@ -310,34 +309,24 @@ def daac(self, daac_short_name: str = "") -> Type[CollectionQuery]:

def temporal(
self,
date_from: Optional[Union[str, dt.datetime]] = None,
date_to: Optional[Union[str, dt.datetime]] = None,
date_from: Optional[Union[str, dt.date]] = None,
date_to: Optional[Union[str, dt.date]] = None,
Comment on lines +411 to +412
Copy link
Collaborator

@jhkennedy jhkennedy Apr 19, 2024 •
edited by chuckwondo
Loading

Choose a reason for hiding this comment

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

Suggested change
date_from: Optional[Union[str, dt.date]] = None,
date_to: Optional[Union[str, dt.date]] = None,
date_from: Optional[Union[str, dt.date, dt.datetime]] = None,
date_to: Optional[Union[str, dt.date, dt.datetime]] = None,

I know it's not necessary or even correct since datetime is a subclass of date, but I think from a documentation standpoint this will be clearer to users.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently the type hint does not effect the documentation, because the docstring includes a "type". Do you want that part removed from the docstring?

If type hints are being included to help developers, then I would want them to be technically correct. If type hints are being included to help users ... well, I fell like that's a little lazy actually. Users don't benefit a whole lot from "dt.date" without knowing what "dt" is. And do they know what "Optional[Union[...]]" means? Relying on auto documentation is nice ... if it's good documentation.

You are welcome to modify however you think its best.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Personally, when type annotations are provided, I drop type information from the docstrings. Otherwise, it's all too easy for the docstrings to become out of sync with the type annotations, leading to confusion, misinformation, and frustration. Modern doc generators, including mkdocs, will make use of the type annotations to put such information in the docs. Doc generators should also use canonical names for things as well, meaning that even though the code may refer to the datetime module as dt, the docs will show the name datetime, so there's no need for the reader to know about any aliasing within the code.

exclude_boundary: bool = False,
) -> Type[CollectionQuery]:
"""Filter by an open or closed date range. Dates can be provided as datetime objects
or ISO 8601 formatted strings. Multiple ranges can be provided by successive calls
to this method before calling execute().

Parameters:
date_from (String or Datetime object): earliest date of temporal range
date_to (String or Datetime object): latest date of temporal range
exclude_boundary (Boolean): whether or not to exclude the date_from/to in the matched range.
"""
DEFAULT = dt.datetime(1979, 1, 1)
if date_from is not None and not isinstance(date_from, dt.datetime):
try:
date_from = parser.parse(date_from, default=DEFAULT).isoformat() + "Z"
except Exception:
print("The provided start date was not recognized")
date_from = ""

if date_to is not None and not isinstance(date_to, dt.datetime):
try:
date_to = parser.parse(date_to, default=DEFAULT).isoformat() + "Z"
except Exception:
print("The provided end date was not recognized")
date_to = ""
"""Filter by an open or closed date range. Dates can be provided as date objects
or ISO 8601 strings. Multiple ranges can be provided by successive method calls.

???+ Tip
Giving either `datetime.date(YYYY, MM, DD)` or `"YYYY-MM-DD"` as the `date_to`
parameter includes that entire day (i.e. the time is set to `23:59:59`).
Using `datetime.datetime(YYYY, MM, DD)` is different, because `datetime.datetime`
objects have `00:00:00` as their built-in default.
itcarroll marked this conversation as resolved.
Show resolved Hide resolved

Parameters:
date_from (String or Date): start of temporal range
date_to (String or Date): end of temporal range
exclude_boundary (Boolean): whether to exclude the date_from and date_to in the matched range.
"""
super().temporal(date_from, date_to, exclude_boundary)
return self

Expand Down Expand Up @@ -616,34 +605,24 @@ def debug(self, debug: bool = True) -> Type[GranuleQuery]:

def temporal(
self,
date_from: Optional[Union[str, dt.datetime]] = None,
date_to: Optional[Union[str, dt.datetime]] = None,
date_from: Optional[Union[str, dt.date]] = None,
date_to: Optional[Union[str, dt.date]] = None,
Comment on lines +803 to +804
Copy link
Collaborator

@jhkennedy jhkennedy Apr 19, 2024 •
edited by chuckwondo
Loading

Choose a reason for hiding this comment

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

Suggested change
date_from: Optional[Union[str, dt.date]] = None,
date_to: Optional[Union[str, dt.date]] = None,
date_from: Optional[Union[str, dt.date, dt.datetime]] = None,
date_to: Optional[Union[str, dt.date, dt.datetime]] = None,

exclude_boundary: bool = False,
) -> Type[GranuleQuery]:
"""Filter by an open or closed date range.
Dates can be provided as a datetime objects or ISO 8601 formatted strings. Multiple
ranges can be provided by successive calls to this method before calling execute().

Parameters:
date_from: earliest date of temporal range
date_to: latest date of temporal range
exclude_boundary: whether to exclude the date_from/to in the matched range
"""
DEFAULT = dt.datetime(1979, 1, 1)
if date_from is not None and not isinstance(date_from, dt.datetime):
try:
date_from = parser.parse(date_from, default=DEFAULT).isoformat() + "Z"
except Exception:
print("The provided start date was not recognized")
date_from = ""

if date_to is not None and not isinstance(date_to, dt.datetime):
try:
date_to = parser.parse(date_to, default=DEFAULT).isoformat() + "Z"
except Exception:
print("The provided end date was not recognized")
date_to = ""
"""Filter by an open or closed date range. Dates can be provided as date objects
or ISO 8601 strings. Multiple ranges can be provided by successive method calls.

???+ Tip
Giving either `datetime.date(YYYY, MM, DD)` or `"YYYY-MM-DD"` as the `date_to`
parameter includes that entire day (i.e. the time is set to `23:59:59`).
Using `datetime.datetime(YYYY, MM, DD)` is different, because `datetime.datetime`
objects have `00:00:00` as their built-in default.

Parameters:
date_from (String or Date): start of temporal range
date_to (String or Date): end of temporal range
exclude_boundary (Boolean): whether to exclude the date_from and date_to in the matched range.
"""
super().temporal(date_from, date_to, exclude_boundary)
return self

Expand Down
Loading
Loading