CLN: datetimelike arrays: isort, small reorg #23587

jbrockmendel · 2018-11-08T23:43:50Z

Big diff, few real changes.

Implement __repr__ for DTA/TDA. Shared with PeriodArray implementation. Added truncation so we don't print all N elements. Adds tests for all three subclasses. Creates dedicated test files for test_datetimes and test_timedeltas

Some PeriodArray methods got shuffled into weird places, so this puts them back.
Collects wrapping-properties in DatetimeIndex.
isort on indexes/datetimelike, datetimes, timedeltas

…range things that got shuffled in rebasing

pep8speaks · 2018-11-08T23:43:58Z

Hello @jbrockmendel! Thanks for submitting the PR.

There are no PEP8 issues in the file pandas/core/arrays/datetimelike.py !
There are no PEP8 issues in the file pandas/core/arrays/datetimes.py !
There are no PEP8 issues in the file pandas/core/arrays/period.py !
There are no PEP8 issues in the file pandas/core/arrays/timedeltas.py !
There are no PEP8 issues in the file pandas/core/indexes/datetimelike.py !
There are no PEP8 issues in the file pandas/core/indexes/datetimes.py !
There are no PEP8 issues in the file pandas/core/indexes/period.py !
There are no PEP8 issues in the file pandas/core/indexes/timedeltas.py !
There are no PEP8 issues in the file pandas/tests/arrays/test_datetimelike.py !
There are no PEP8 issues in the file pandas/tests/arrays/test_datetimes.py !
There are no PEP8 issues in the file pandas/tests/arrays/test_period.py !
There are no PEP8 issues in the file pandas/tests/arrays/test_timedeltas.py !

pandas/core/arrays/datetimelike.py

pandas/core/arrays/period.py

codecov · 2018-11-09T01:20:49Z

Codecov Report

❗ No coverage uploaded for pull request base (master@592fd64). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master   #23587   +/-   ##
=========================================
  Coverage          ?   92.24%           
=========================================
  Files             ?      161           
  Lines             ?    51316           
  Branches          ?        0           
=========================================
  Hits              ?    47337           
  Misses            ?     3979           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.63% <100%> (?)`
#single	`42.29% <79.59%> (?)`

Impacted Files	Coverage Δ
pandas/core/arrays/timedeltas.py	`95.08% <ø> (ø)`
pandas/core/arrays/datetimes.py	`98.44% <ø> (ø)`
pandas/core/indexes/period.py	`93.04% <ø> (ø)`
pandas/core/arrays/datetimelike.py	`95.92% <100%> (ø)`
pandas/core/indexes/datetimelike.py	`98.01% <100%> (ø)`
pandas/core/indexes/timedeltas.py	`89.26% <100%> (ø)`
pandas/core/arrays/period.py	`98.49% <100%> (ø)`
pandas/core/indexes/datetimes.py	`96.12% <100%> (ø)`

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 592fd64...c7211f7. Read the comment docs.

pandas/core/arrays/datetimelike.py

TomAugspurger

LGTM at a glance. What's the motivation for aliasing DatetimeArrayMixin to DatetimeArray, rather than doing the rename all at once?

pandas/core/arrays/timedeltas.py

jbrockmendel · 2018-11-09T03:58:28Z

What's the motivation for aliasing DatetimeArrayMixin to DatetimeArray, rather than doing the rename all at once?

In several branches where I have tried doing it all-at-once the diffs become difficult for even me to read. Getting this out of the way early saves some trouble.

jorisvandenbossche

Some minor comments

pandas/core/arrays/datetimelike.py

jorisvandenbossche · 2018-11-09T13:08:58Z

pandas/core/arrays/period.py

@@ -286,10 +282,6 @@ def _ndarray_values(self):
        # Ordinals
        return self._data

-    @property
-    def asi8(self):
-        return self._data


but for datetimes the _data is not integer?

The inherited implementation returns self._data.view('i8'). The view is redundant for PeriodArray, but still correct

jorisvandenbossche · 2018-11-09T13:19:41Z

pandas/core/indexes/datetimes.py

-    microsecond = wrap_field_accessor(DatetimeArrayMixin.microsecond)
-    nanosecond = wrap_field_accessor(DatetimeArrayMixin.nanosecond)
-    weekofyear = wrap_field_accessor(DatetimeArrayMixin.weekofyear)
+    # override DatetimeLikeArrayMixin.__repr__


can you add a "TODO remove" ?

Sure, but FWIW that's the point of collecting things in this section

jorisvandenbossche · 2018-11-09T13:20:40Z

pandas/core/indexes/period.py

@@ -256,8 +256,15 @@ def _simple_new(cls, values, name=None, freq=None, **kwargs):
        result._reset_identity()
        return result

+    # ------------------------------------------------------------------------
+    # Wrapping PeriodArray


What is this section for?

Collecting things that will soon be changed by the inheritance/composition in one place. For PeriodIndex its a pretty small set

jorisvandenbossche · 2018-11-09T13:23:01Z

pandas/tests/arrays/test_datetimes.py

+
+
+class TestDatetimeArray(object):
+    def test_repr(self):


Do we want to put this in a class? Or do like in PeriodArray tests?

It would be more idiomatic to use functions, unless we need the class org for a particular reason. The same goes for all of the other add tests.

…ike_ea3

jbrockmendel · 2018-11-09T18:22:38Z

Are we OK with merging this and then shortly thereafter getting rid of the __repr__ here and replace it with #23601? Or should I remove repr and tests from this PR?

TomAugspurger · 2018-11-09T21:08:19Z

I'm going to revert the repr changes if that's OK. We'll need to change them eventually, when the name changes from ArrayMixin to just Array, and since they aren't inheriting from ExtensionArray yet the formatting on #23601 won't quite work..

TomAugspurger · 2018-11-09T21:22:59Z

Converted the new array repr tests to xfails. We'll need to be a bit careful with those once we DatetimeArrayMixin inherits from DatetimeArray. I may not have gotten the future formatting 100% correct.

jbrockmendel · 2018-11-09T22:06:54Z

Sounds good, thanks.

…ike_ea3

jbrockmendel · 2018-11-10T03:07:12Z

Fixed I sort that broke travis, hopefully got all of them

pandas/core/indexes/datetimes.py

pandas/tests/arrays/test_datetimelike.py

setup.cfg

…ike_ea3

jbrockmendel · 2018-11-11T01:57:29Z

Travis fail is Hypothesis

…ike_ea3

pandas/core/indexes/datetimes.py

pandas/tests/arrays/test_datetimes.py

pandas/tests/arrays/test_timedeltas.py

jbrockmendel · 2018-11-11T17:13:26Z

I see you have added [_timezone] [...] what is the point of _timezones when _tz already does this?

I didn't add this, has existed for a while. No idea what the original reasoning was. Maybe just an alias?

…ike_ea3

jreback · 2018-11-11T18:11:19Z

@jbrockmendel #23587 (comment)

ok. can you create an issue about seeing what that is about. it seems pretty confusing (though its actually a nice name).

jreback · 2018-11-11T18:12:52Z

lgtm. ping on green (circle seems to have failed <:)

…ike_ea3

jorisvandenbossche · 2018-11-12T10:44:47Z

pandas/core/indexes/period.py

+    # Wrapping PeriodArray
+
+    # override DatetimeLikeArrayMixin.__repr__
+    __repr__ = Index.__repr__


This can be removed now I think? (now the repr changes are reversed?)

jorisvandenbossche · 2018-11-12T11:42:57Z

Thanks @jbrockmendel !

* upstream/master: BUG: Don't over-optimize memory with jagged CSV (pandas-dev#23527) DEPR: Deprecate usecols as int in read_excel (pandas-dev#23635) More helpful Stata string length error. (pandas-dev#23629) BUG: astype fill_value for SparseArray.astype (pandas-dev#23547) CLN: datetimelike arrays: isort, small reorg (pandas-dev#23587) CI: Check in the CI that assert_raises_regex is not being used (pandas-dev#23627) CLN:Remove unused **kwargs from user facing methods (pandas-dev#23249) DOC: Enhancing pivot / reshape docs (pandas-dev#21038) TST: Fix xfailing DataFrame arithmetic tests by transposing (pandas-dev#23620)

…fixed * upstream/master: DOC: avoid SparseArray.take error (pandas-dev#23637) CLN: remove incorrect usages of com.AbstractMethodError (pandas-dev#23625) DOC: Adding validation of the section order in docstrings (pandas-dev#23607) BUG: Don't over-optimize memory with jagged CSV (pandas-dev#23527) DEPR: Deprecate usecols as int in read_excel (pandas-dev#23635) More helpful Stata string length error. (pandas-dev#23629) BUG: astype fill_value for SparseArray.astype (pandas-dev#23547) CLN: datetimelike arrays: isort, small reorg (pandas-dev#23587) CI: Check in the CI that assert_raises_regex is not being used (pandas-dev#23627) CLN:Remove unused **kwargs from user facing methods (pandas-dev#23249)

jbrockmendel added 4 commits November 8, 2018 14:09

import mixins without mixins in names

e3267bc

isort timedeltas, datetimelike, datetime index files

d4bde65

Section header

a2754c1

implement repr, with tests, truncate long reprs for periodarray, rear…

e81f5d6

…range things that got shuffled in rebasing

jbrockmendel commented Nov 8, 2018

View reviewed changes

pandas/core/arrays/datetimelike.py Outdated Show resolved Hide resolved

jbrockmendel commented Nov 8, 2018

View reviewed changes

pandas/core/arrays/period.py Show resolved Hide resolved

isort

1647ea2

TomAugspurger reviewed Nov 9, 2018

View reviewed changes

pandas/core/arrays/datetimelike.py Outdated Show resolved Hide resolved

jreback requested changes Nov 9, 2018

View reviewed changes

pandas/core/arrays/datetimelike.py Outdated Show resolved Hide resolved

TomAugspurger reviewed Nov 9, 2018

View reviewed changes

pandas/core/arrays/timedeltas.py Show resolved Hide resolved

jbrockmendel mentioned this pull request Nov 9, 2018

See if existing formatting code can be re-used for EA repr #23590

Closed

jorisvandenbossche reviewed Nov 9, 2018

View reviewed changes

Merge branch 'master' of https://github.com/pandas-dev/pandas into dl…

c99ca32

…ike_ea3

TomAugspurger mentioned this pull request Nov 9, 2018

Add default repr for EAs #23601

Merged

remove repr changes

db1d446

jbrockmendel added 2 commits November 9, 2018 14:08

Merge branch 'master' of https://github.com/pandas-dev/pandas into dl…

1355f59

…ike_ea3

isort fixup

14d5e76

jreback requested changes Nov 10, 2018

View reviewed changes

pandas/core/indexes/datetimes.py Show resolved Hide resolved

pandas/tests/arrays/test_datetimelike.py Show resolved Hide resolved

setup.cfg Show resolved Hide resolved

Merge branch 'master' of https://github.com/pandas-dev/pandas into dl…

a0c7bf4

…ike_ea3

gfyoung added Datetime Datetime data dtype Output-Formatting __repr__ of pandas objects, to_string Refactor Internal refactoring of code labels Nov 11, 2018

typo fixup

dd85053

Merge branch 'master' of https://github.com/pandas-dev/pandas into dl…

e0c3fcf

…ike_ea3

jreback requested changes Nov 11, 2018

View reviewed changes

pandas/core/indexes/datetimes.py Show resolved Hide resolved

pandas/tests/arrays/test_datetimes.py Outdated Show resolved Hide resolved

pandas/tests/arrays/test_timedeltas.py Outdated Show resolved Hide resolved

jbrockmendel added 2 commits November 11, 2018 09:14

Merge branch 'master' of https://github.com/pandas-dev/pandas into dl…

6fe6496

…ike_ea3

remove repr tests

469ac83

jreback added this to the 0.24.0 milestone Nov 11, 2018

fixup imports

65209f1

jreback approved these changes Nov 11, 2018

View reviewed changes

jbrockmendel mentioned this pull request Nov 11, 2018

Why does DatetimeIndex have _timezone in addition to _tz? #23624

Closed

jbrockmendel added 2 commits November 11, 2018 13:54

flake8 fixups

0ada030

Merge branch 'master' of https://github.com/pandas-dev/pandas into dl…

58dedcf

…ike_ea3

jbrockmendel mentioned this pull request Nov 11, 2018

BUG/REF: TimedeltaIndex.__new__ #23539

Merged

jbrockmendel added 2 commits November 11, 2018 15:58

docstring suggestions

0a17757

fixup unused import

4ac9576

jorisvandenbossche reviewed Nov 12, 2018

View reviewed changes

undo remaining repr changes

c7211f7

jorisvandenbossche changed the title ~~REF/ENH: __repr__ for DTA/TDA, tests, isort~~ CLN: datetimelike arrays: isort, small reorg Nov 12, 2018

jorisvandenbossche merged commit 3592a46 into pandas-dev:master Nov 12, 2018

jbrockmendel deleted the dlike_ea3 branch November 12, 2018 14:43

JustinZhengBC pushed a commit to JustinZhengBC/pandas that referenced this pull request Nov 14, 2018

CLN: datetimelike arrays: isort, small reorg (pandas-dev#23587)

d8826bf

tm9k1 pushed a commit to tm9k1/pandas that referenced this pull request Nov 19, 2018

CLN: datetimelike arrays: isort, small reorg (pandas-dev#23587)

baa9989

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

CLN: datetimelike arrays: isort, small reorg (pandas-dev#23587)

72506d0

Pingviinituutti pushed a commit to Pingviinituutti/pandas that referenced this pull request Feb 28, 2019

CLN: datetimelike arrays: isort, small reorg (pandas-dev#23587)

4f86a12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CLN: datetimelike arrays: isort, small reorg #23587

CLN: datetimelike arrays: isort, small reorg #23587

jbrockmendel commented Nov 8, 2018

pep8speaks commented Nov 8, 2018

codecov bot commented Nov 9, 2018 •

edited

Loading

TomAugspurger left a comment

jbrockmendel commented Nov 9, 2018

jorisvandenbossche left a comment

jorisvandenbossche Nov 9, 2018

jbrockmendel Nov 9, 2018

jorisvandenbossche Nov 9, 2018

jbrockmendel Nov 9, 2018

jorisvandenbossche Nov 9, 2018

jbrockmendel Nov 9, 2018

jorisvandenbossche Nov 9, 2018

gfyoung Nov 11, 2018

jbrockmendel commented Nov 9, 2018

TomAugspurger commented Nov 9, 2018

TomAugspurger commented Nov 9, 2018

jbrockmendel commented Nov 9, 2018

jbrockmendel commented Nov 10, 2018

jbrockmendel commented Nov 11, 2018

jbrockmendel commented Nov 11, 2018

jreback commented Nov 11, 2018

jreback commented Nov 11, 2018

jorisvandenbossche Nov 12, 2018

jorisvandenbossche commented Nov 12, 2018

CLN: datetimelike arrays: isort, small reorg #23587

CLN: datetimelike arrays: isort, small reorg #23587

Conversation

jbrockmendel commented Nov 8, 2018

pep8speaks commented Nov 8, 2018

codecov bot commented Nov 9, 2018 • edited Loading

Codecov Report

TomAugspurger left a comment

Choose a reason for hiding this comment

jbrockmendel commented Nov 9, 2018

jorisvandenbossche left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrockmendel commented Nov 9, 2018

TomAugspurger commented Nov 9, 2018

TomAugspurger commented Nov 9, 2018

jbrockmendel commented Nov 9, 2018

jbrockmendel commented Nov 10, 2018

jbrockmendel commented Nov 11, 2018

jbrockmendel commented Nov 11, 2018

jreback commented Nov 11, 2018

jreback commented Nov 11, 2018

Choose a reason for hiding this comment

jorisvandenbossche commented Nov 12, 2018

codecov bot commented Nov 9, 2018 •

edited

Loading