-
Notifications
You must be signed in to change notification settings - Fork 30
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
pandas 2.0 support #300
pandas 2.0 support #300
Conversation
@@ -9,7 +9,7 @@ | |||
|
|||
import httpx | |||
import pytz | |||
from pandas._libs.tslibs.parsing import parse_time_string | |||
from pandas import to_datetime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome, thanks for the quick fix !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about extracting the import into a function and then tweaking execution on import errors?
Then we could probably revert the new pin.
def dang_datetimes(date_time: Union[datetime, str], dayfirst: bool=False, yearfirst: bool=False):
try:
from pandas import to_datetime
return to_datetime(datetime, dayfirst=dayfirst, yearfirst=yearfalse).to_pydatetime()
except ImportError:
from pandas._libs.tslibs.parsing import parse_time_string
return parse_time_string(date_time)[0]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then we could probably revert the new pin.
The new pin (>=1.5.3
) is mostly b/c I was lazy and creating old envs to test but I'm 99% sure we can go as low as the old one (>=0.20.3
) with the current code. I can get back to this in the afternoon and do more tests, hold on...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the quick response to this issue @ocefpaf !
- a patch is on its way at ioos/erddapy#300 and conda-forge/conda-forge-repodata-patches-feedstock#426
- a patch is on its way at ioos/erddapy#300 and conda-forge/conda-forge-repodata-patches-feedstock#426
The failures are due to a deprecation warning in xarray. I like the idea of making deprecation warnings errors in CIs but I couldn't figure out to limit those to erddapy only. |
Maybe |
That works but the config is awkward b/c we need to do it for every third party library. Maybe there is a way to ensure the rule is enforced only for the main lib. |
|
That did the trick! Thanks!! |
Closes #299
In this PR:
dayfirst
andyearfirst
options toparse_dates
. Only the default work when calling it internally but a user has the option to convert the date themselves this way.