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

IOC: hour component is ignored in endtime #96

Closed
pmav99 opened this issue Jul 25, 2023 · 0 comments · Fixed by #97
Closed

IOC: hour component is ignored in endtime #96

pmav99 opened this issue Jul 25, 2023 · 0 comments · Fixed by #97
Assignees

Comments

@pmav99
Copy link
Member

pmav99 commented Jul 25, 2023

our resolve_date() function is accepting hours/minutes/seconds but it silently ignores them. This can be seen in the following examples.

When we use a specific "Date" then, correctly, endtime is assumed to be at midnight and this is the behavior we get:

data = ioc.get_ioc_station_data("scor", endtime=pd.Timestamp("2023-05-04"), period=0.1)

data.time.min().time()  # 21:37
data.time.max().time()  # 23:57

If we specify a different hour though, e.g. noon then the hours are ignored:

data = ioc.get_ioc_station_data("scor", endtime=pd.Timestamp("2023-05-04T12:00:00"), period=0.1)

data.time.min().time()  # We still get 21:37 instead of ~09:37
data.time.max().time()  # We still get 23:57 instead of ~11:57 

This is the relevant code:

searvey/searvey/utils.py

Lines 43 to 48 in c3f547b

def resolve_date(date: DateLike) -> datetime.date:
if date == TODAY:
date = datetime.date.today()
else:
date = pd.Timestamp(date).date()
return date

@pmav99 pmav99 self-assigned this Jul 25, 2023
pmav99 added a commit to pmav99/searvey that referenced this issue Jul 25, 2023
This involves several changes:

1. We rename `resolve_date` to `resolve_timestamp` since we no longer only deal with dates
2. The signature and the type of returned objects in`resolve_timestamp()` has changed.
    More specifically the function became quite a bit more capable and complex.
    It gained extra arguments that allow it to return both timezone-aware and
    naive Timestamps.
3. We change the Literal value `TODAY` to `now`

Fixes oceanmodeling#96
pmav99 added a commit to pmav99/searvey that referenced this issue Jul 26, 2023
This involves several changes:

1. We rename `resolve_date` to `resolve_timestamp` since we no longer only deal with dates
2. The signature and the type of returned objects in`resolve_timestamp()` has changed.
    More specifically the function became quite a bit more capable and complex.
    It gained extra arguments that allow it to return both timezone-aware and
    naive Timestamps.
3. We change the Literal value `TODAY` to `now`

Fixes oceanmodeling#96
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 a pull request may close this issue.

1 participant