Skip to content

review how we duck-type non-numpy inputs #16402

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

Open
tacaswell opened this issue Feb 3, 2020 · 5 comments
Open

review how we duck-type non-numpy inputs #16402

tacaswell opened this issue Feb 3, 2020 · 5 comments

Comments

@tacaswell
Copy link
Member

We currently do a fair amount of ad-hoc normalization / edge case handling for when we get pandas (or xarray or TF or ...) objects that are close to numpy but not quite (for example #16347 which uses a warning context manager which is extra not-thread-safe in other places we check of obj.values).

Can we centralize / streamline this logic?

@tacaswell tacaswell added this to the v3.3.0 milestone Feb 3, 2020
@tacaswell
Copy link
Member Author

also xref: #16400

@QuLogic QuLogic modified the milestones: v3.3.0, v3.4.0 May 22, 2020
@QuLogic QuLogic modified the milestones: v3.4.0, v3.5.0 Jan 27, 2021
@jklymak
Copy link
Member

jklymak commented May 3, 2021

We don't really ever get complaints from TF or xarray, so the practical question is how much do we special-case pandas, and who will set up the tests, and lead the charge on integration? This is, in my opinion, hampered by not having anyone on the lead-dev team who regularly uses pandas. It would be nice to try and entrain someone who did, who could advocate for changes and/or interface with the pandas project (who I have found are great to work with, but you know, are pushing their own things forward).

@timhoffm
Copy link
Member

timhoffm commented May 3, 2021

.values is not recommended anymore in pandas: https://pandas.pydata.org/docs/reference/api/pandas.Series.values.html / https://pandas.pydata.org/docs/reference/api/pandas.DataFrame.values.html. Probably to_numpy() is the way to go.

@jklymak
Copy link
Member

jklymak commented May 3, 2021

Yeah, unfortunately, thats not universal. xarray still uses .values.

@QuLogic QuLogic modified the milestones: v3.5.0, v3.6.0 Sep 25, 2021
@oscargus
Copy link
Member

Which libraries are we talking about? pandas, xarray, polars, tensorflow(?), ...

I've added a test for xarray in #22560, which also centralizes this, but probably makes sense to add tests for all libraries that is of interest (there was one for pandas). (polars can not yet be installed with conda.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants