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

MAINT: refactor from_items() using from_dict() #22094

Closed
wants to merge 1 commit into from

Conversation

jzwinck
Copy link
Contributor

@jzwinck jzwinck commented Jul 28, 2018

This removes the deprecation warnings introduced in #18262,
by reimplementing DataFrame.from_items() in the recommended
way using DataFrame.from_dict() and collections.OrderedDict.

This eliminates the maintenance burden of separate code for
from_items(), while allowing existing uses to keep working.

A small cleanup can be done once #8425 is fixed.

This removes the deprecation warnings introduced in pandas-dev#18262,
by reimplementing DataFrame.from_items() in the recommended
way using DataFrame.from_dict() and collections.OrderedDict.

This eliminates the maintenance burden of separate code for
from_items(), while allowing existing uses to keep working.

A small cleanup can be done once pandas-dev#8425 is fixed.
@jzwinck
Copy link
Contributor Author

jzwinck commented Jul 28, 2018

CI is reporting an error in TestHDFStore.test_calendar_roundtrip_issue but it is not caused by this PR (other Pandas PRs report the same error).

@jreback
Copy link
Contributor

jreback commented Jul 28, 2018

did u not see my comment on the issue?

pandas already has way too many apis

just because X number of people use something is only 1 factor in a deprecation

@jreback jreback closed this Jul 28, 2018
@jzwinck
Copy link
Contributor Author

jzwinck commented Jul 28, 2018

@jreback I did see your comment, which read:

there is a burden to maintain apis and code why should this be any different from other ‘highly used code’

This PR makes from_items() a simple wrapper around from_dict(), which surely imposes less burden than repeatedly answering questions from people whose code using from_items() now causes warnings (and soon, failures). You might be under the impression that everyone can just replace from_items(x, ...) with from_dict(OrderedDict(x), ...), but as this PR shows it is not quite so simple. In fact there are three different "modes", one of which actually produces the wrong result if used without my wrapper (due to #8425).

I realize that you strongly prefer to eliminate code. This PR reduces code relative to 0.22 and even more so when compared with master. It adds a small amount of simple code relative to your preferred end state, but the benefit is that it allows thousands of existing applications and notebooks to continue working.

@jorisvandenbossche @gfyoung @chris-b1 @reidy-p @wesm I do not agree with the closure of this PR without a substantive discussion. I do not know whether Pandas is unilaterally ruled by one person, but I would appreciate your consideration of this PR to avoid breaking a large number of programs. I believe Pandas has a long future ahead, but this API brinksmanship erodes confidence.

@jorisvandenbossche
Copy link
Member

Let's keep the discussion on the issue #21850 (but @jzwinck your PR is certainly useful for illustration purposes for the discussion as well), I'll reopen the issue.

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.

Un-deprecate DataFrame.from_items()
3 participants