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

Fix Issue #21336: Defeault behavior for resolution property in Timestamps #21365

Closed
wants to merge 15 commits into from

Conversation

Projects
None yet
4 participants
@drewmassey
Copy link

commented Jun 7, 2018

  • closes #21336
  • tests added / passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

@drewmassey drewmassey changed the title Issue 21336 Fix Issue #21336: Defeault behavior for resolution property in Timestamps Jun 7, 2018

@mroeschke

This comment has been minimized.

Copy link
Member

commented Jun 7, 2018

Looks good overall! Looks like the CI failed due to some linting errors.

@@ -116,3 +116,4 @@ Other

- Tab completion on :class:`Index` in IPython no longer outputs deprecation warnings (:issue:`21125`)
- Bug preventing pandas from being importable with -OO optimization (:issue:`21071`)
- Timestamp resolution returns a :class:`Timedelta` rather than a normal `timedelta` object (:issue:`21336`)

This comment has been minimized.

Copy link
@mroeschke

mroeschke Jun 7, 2018

Member

I think the main point of the issue was that before the stated resolution was 1 microsecond while in actuality its 1 nanosecond (but we can't use timedelta to express that because it doesn't support nanoseconds)

This comment has been minimized.

Copy link
@drewmassey

drewmassey Jun 8, 2018

Author

Resolved in commit c53f2c2

@codecov

This comment has been minimized.

Copy link

commented Jun 7, 2018

Codecov Report

Merging #21365 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #21365      +/-   ##
==========================================
- Coverage   91.89%   91.89%   -0.01%     
==========================================
  Files         153      153              
  Lines       49600    49596       -4     
==========================================
- Hits        45580    45576       -4     
  Misses       4020     4020
Flag Coverage Δ
#multiple 90.29% <ø> (-0.01%) ⬇️
#single 41.86% <ø> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/base.py 96.61% <0%> (-0.01%) ⬇️
pandas/core/frame.py 97.22% <0%> (-0.01%) ⬇️
pandas/core/indexing.py 93.55% <0%> (ø) ⬆️
pandas/core/reshape/pivot.py 97.03% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92e9882...f28dc5d. Read the comment docs.

@mroeschke mroeschke added the Timeseries label Jun 7, 2018

@drewmassey

This comment has been minimized.

Copy link
Author

commented Jun 8, 2018

@mroeschke This passed linting on my local machine, but now there is a simple conflict to resolve under what's new that I can't do because I don't have repo write access. Is this something you can resolve, or point me to someone who can? Thanks.

@mroeschke

This comment has been minimized.

Copy link
Member

commented Jun 8, 2018

It's possible to fix the merge conflict locally and then re-push the resolution back to this branch. The general flow instructions can be found here: https://pandas.pydata.org/pandas-docs/stable/contributing.html#updating-your-pull-request

Once you execute the git merge upstream/master command in your console, you should be prompted to fix the merge conflict with the whatsnew document. Then you can commit the change and push back to this branch.

@@ -131,3 +131,5 @@ Bug Fixes
**Other**

- Tab completion on :class:`Index` in IPython no longer outputs deprecation warnings (:issue:`21125`)
- Bug preventing pandas from being importable with -OO optimization (:issue:`21071`)

This comment has been minimized.

Copy link
@mroeschke

mroeschke Jun 8, 2018

Member

Looks like this already in the **Other Fixes** section.

@jbrockmendel

This comment has been minimized.

Copy link
Member

commented Jun 10, 2018

LGTM. Only nitpick would be with the wording "native pandas", but I don't have any better ideas.

@mroeschke
Copy link
Member

left a comment

LGTM. Is this appropriate for 0.23.1 @jreback?

@drewmassey

This comment has been minimized.

Copy link
Author

commented Jun 12, 2018

Could I get a resolution on this from a contributor? Happy to fiddle the PR if necessary but don't want it to get stale.

@mroeschke mroeschke added this to the 0.23.2 milestone Jun 12, 2018

@@ -132,4 +132,5 @@ Bug Fixes
**Other**

- Tab completion on :class:`Index` in IPython no longer outputs deprecation warnings (:issue:`21125`)
- Timestamp resolution returns a :class:`Timedelta` (in nanoseconds) rather than a normal `timedelta` object (in microseconds) (:issue:`21336`)

This comment has been minimized.

Copy link
@mroeschke

mroeschke Jun 12, 2018

Member

Would you mind moving this entry to the v0.23.2.txt whatsnew? The 0.23.1 release is going out today or tomorrow I believe, and it appears the backports have already been established. Thanks for your patience.

@mroeschke

This comment has been minimized.

Copy link
Member

commented Jun 12, 2018

Hmm looks like this branch picked up the commits of your other PR #21444. You'll need to revert the HEAD of this branch to a prior commit.

@jreback

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2018

can you rebase

@jreback jreback modified the milestones: 0.23.2, 0.23.3 Jun 26, 2018

@jbrockmendel jbrockmendel referenced this pull request Jul 11, 2018

Merged

CLN: miscellaneous cleanups / fixes #21861

3 of 3 tasks complete
@mroeschke

This comment has been minimized.

Copy link
Member

commented Jul 12, 2018

Looks like #21861 resolved this original issue (#21336). Would still greatly appreciate #21444!

@mroeschke mroeschke closed this Jul 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.