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

pandas 2.0 support #300

Merged
merged 6 commits into from
Apr 6, 2023
Merged

pandas 2.0 support #300

merged 6 commits into from
Apr 6, 2023

Conversation

ocefpaf
Copy link
Member

@ocefpaf ocefpaf commented Apr 4, 2023

Closes #299

In this PR:

  • We are still relying on a private function but that code is the best there is :-/
  • We are pinning to pandas >=2.0. Should be OK b/c we are patching the previous released erddapies to work with older pandas only (conda-forge), there is nothing we can do for PyPI;
  • Re-added the tag regex that was accidentally removed;
  • Added the dayfirst and yearfirst options to parse_dates. Only the default work when calling it internally but a user has the option to convert the date themselves this way.

@@ -9,7 +9,7 @@

import httpx
import pytz
from pandas._libs.tslibs.parsing import parse_time_string
from pandas import to_datetime
Copy link
Member Author

Choose a reason for hiding this comment

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

@gmaze I should've done this a long time ago :-)

Thanks for the ping in

this looks like a recurrent issue for erddapy (#128 #34 #240 )

Copy link

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 !

Copy link
Contributor

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]

Copy link
Member Author

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...

requirements.txt Outdated Show resolved Hide resolved
@ocefpaf ocefpaf requested a review from abkfenris April 4, 2023 20:28
Copy link

@gmaze gmaze left a 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 !

gmaze added a commit to euroargodev/argopy that referenced this pull request Apr 5, 2023
gmaze added a commit to euroargodev/argopy that referenced this pull request Apr 5, 2023
@ocefpaf
Copy link
Member Author

ocefpaf commented Apr 5, 2023

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.

@abkfenris
Copy link
Contributor

Maybe ignore:::xarray?

@ocefpaf
Copy link
Member Author

ocefpaf commented Apr 5, 2023

Maybe ignore:::xarray?

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.

@abkfenris
Copy link
Contributor

error:::erddapy.* or similar?

@ocefpaf
Copy link
Member Author

ocefpaf commented Apr 6, 2023

error:::erddapy.* or similar?

That did the trick! Thanks!!

@ocefpaf ocefpaf merged commit f4bece4 into ioos:main Apr 6, 2023
@ocefpaf ocefpaf deleted the pandas_2 branch April 6, 2023 14:39
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 this pull request may close these issues.

Impossible to import erddapy with pandas 2.0
3 participants