Skip to content

BUG: fix alignment in series ops (GH14227) #14230

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

Merged

Conversation

jorisvandenbossche
Copy link
Member

@jorisvandenbossche jorisvandenbossche commented Sep 15, 2016

@codecov-io
Copy link

codecov-io commented Sep 15, 2016

Current coverage is 85.26% (diff: 76.92%)

Merging #14230 into master will decrease coverage by <.01%

@@             master     #14230   diff @@
==========================================
  Files           140        140          
  Lines         50619      50630    +11   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          43161      43168     +7   
- Misses         7458       7462     +4   
  Partials          0          0          

Powered by Codecov. Last update 5033a4a...66d35d1

# if DatetimeIndex have different tz, convert to UTC
left.index = index
right.index = index
if is_datetime64tz_dtype(left.index):
Copy link
Contributor

Choose a reason for hiding this comment

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

should be inside .join not here

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean in align ?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, that's what I mean

@jreback
Copy link
Contributor

jreback commented Sep 20, 2016

@jorisvandenbossche can you update

@jreback jreback modified the milestones: 0.19.0, 0.19.1 Sep 28, 2016
@jorisvandenbossche
Copy link
Member Author

@jreback I moved this to align (and therefore also fixing the bug in align), but it is still not yet that clean ...
So the problem for tz aware indexes is that Index.join does not returns indexers when the result does not need to be reindexed (although in case of tz aware indexes the actual indexes can be different and have to be adapted), and based on that the index of the result is adapted in align or not. So I still have to check for this specific case. I tried to solve it in Index.join itself, but that caused other errors.

@jreback
Copy link
Contributor

jreback commented Sep 30, 2016

if this works (and you want to push in 0.19.0), ok.. maybe just add a TODO / issue to fix/clean later (otherwise better wait for 0.19.1)

@jorisvandenbossche
Copy link
Member Author

Well, it was a hack before, and it is still the exact same hack now, only moved down the stack into align (fixing the behaviour of align as well), so merging for now.

@jorisvandenbossche jorisvandenbossche merged commit 6dcc238 into pandas-dev:master Sep 30, 2016
@jorisvandenbossche jorisvandenbossche modified the milestones: 0.19.0, 0.19.1 Sep 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Labels mismatch when adding series with repeated index values in 0.19rc1
3 participants