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

DEPR: _metadata propagation #52168

Closed
wants to merge 7 commits into from

Conversation

jbrockmendel
Copy link
Member

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

xref #51280

@jbrockmendel
Copy link
Member Author

Mothballing, will revisit in a few months to see if the pro-_metadata folks make progress on getting it fully working.

@jbrockmendel jbrockmendel added the Mothballed Temporarily-closed PR the author plans to return to label May 4, 2023
@jorisvandenbossche
Copy link
Member

Mothballing, will revisit in a few months to see if the pro-_metadata folks make progress on getting it fully working.

It is working ...

@jbrockmendel
Copy link
Member Author

137 xfailed tests in tests.generic.test_finalize say otherwise.

@jorisvandenbossche
Copy link
Member

The existence of a many packages implementing subclasses that happily make use of _metadata says otherwise?

The tests you are referring to use attrs to test this, and not all those tests are relevant for the subclasses using _metadata. For example, describe is xfailing but I would say this is fine to not propagate attrs/metadata. squeeze is xfailing, but that's only because it doesn't actually return a pandas object but a scalar. Some reductions are failing, but I think many subclasses don't care about that use case to preserve the class. Idem for categorical accessors.

So I would say that those xfailed tests are not really an indication of how well this is working in general.

@jbrockmendel
Copy link
Member Author

There are better uses of both our time; I have PRs I've been asked to review, and I've asked you to look at topper's reduction PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mothballed Temporarily-closed PR the author plans to return to
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants