Skip to content

BUG: Too aggressive typing in NDFrame.align #31788

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 9, 2020
Merged

BUG: Too aggressive typing in NDFrame.align #31788

merged 2 commits into from
Feb 9, 2020

Conversation

topper-123
Copy link
Contributor

@topper-123 topper-123 commented Feb 7, 2020 •
edited
Loading

The type checking was too aggressive. right has type Any, so the wrapping in _ensure_type should not be done.

@topper-123 topper-123 changed the title BUG: Too aggressive typing NDFrame.align BUG: Too aggressive typing in NDFrame.align Feb 7, 2020
@jbrockmendel
Copy link
Member

The type checking was too aggressive

Is this in reference to annotations or to runtime type checking?

@topper-123
Copy link
Contributor Author

Is this in reference to annotations or to runtime type checking?

It's type checking, but can spill over to the run time because an assert is used to enforce the type, (that's what the call to ._ensure_type does). Without ._ensure_type the type of left would be Optional[DataFrame] and mypy would complain of operations on None.

@jorisvandenbossche
Copy link
Member

Do any of the other usage of _ensure_types introduced in the PR also need fixing?

@jorisvandenbossche jorisvandenbossche added this to the 1.0.2 milestone Feb 7, 2020
@topper-123
Copy link
Contributor Author

There should`t be more; here I assumed that left and right should be treated alike, which was a wrong idea.

WillAyd
WillAyd approved these changes Feb 8, 2020
View reviewed changes
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

lgtm

WillAyd
WillAyd requested changes Feb 8, 2020
View reviewed changes
Copy link
Member

@WillAyd WillAyd left a comment

Choose a reason for hiding this comment

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

too quick on approval...can you add test?

@WillAyd WillAyd added the Regression Functionality that used to work in a prior pandas version label Feb 8, 2020
@topper-123 topper-123 force-pushed the aggressive_ndframe_align_typing branch from b6cc420 to 0a4e997 Compare February 8, 2020 10:09
@topper-123
Copy link
Contributor Author

Ok, I've added a test.

@jreback jreback merged commit 341a719 into pandas-dev:master Feb 9, 2020
@jreback
Copy link
Contributor

jreback commented Feb 9, 2020

thanks can we open an issue about how to improve / fix _ensure_type to make things like this work?

@meeseeksmachine meeseeksmachine mentioned this pull request Feb 9, 2020
Merged
meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Feb 9, 2020
@topper-123 @meeseeksmachine
Backport PR pandas-dev#31788: BUG: Too aggressive typing in NDFrame.a…
a036b4b 
…lign
@jreback jreback added the Typing type annotations, mypy/pyright type checking label Feb 9, 2020
simonjayhawkins pushed a commit that referenced this pull request Feb 9, 2020
@meeseeksmachine @topper-123
Backport PR #31788: BUG: Too aggressive typing in NDFrame.align (#31825)
d6c8ad5 
Co-authored-by: Terji Petersen <[email protected]>
@topper-123 topper-123 deleted the aggressive_ndframe_align_typing branch February 12, 2020 21:49
@TomAugspurger TomAugspurger mentioned this pull request Mar 2, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WillAyd WillAyd WillAyd requested changes

Assignees
No one assigned
Labels
Regression Functionality that used to work in a prior pandas version Typing type annotations, mypy/pyright type checking
Projects
None yet
Milestone
1.0.2
Development

Successfully merging this pull request may close these issues.

Crash when using align between a Series and a DataFrame and ffill (or bfill) method
5 participants
@topper-123 @jbrockmendel @jorisvandenbossche @jreback @WillAyd