use range in RangeIndex instead of _start etc. #26581

topper-123 · 2019-05-30T20:37:22Z

xref PERF: don't call RangeIndex._data unnecessarily #26565
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

Make RangeIndex use python's range internally, rather than the three scalars _start, _stop and _step.

Python3's range has several nice properties, that were not available in xrange in Python2. For example it's sliceable, and has .index and count methods. These can be used in Pandas, rather than maintaining Pandas-specific code, offering cleaner code and possibly faster operations.

topper-123 · 2019-05-30T20:39:10Z

pandas/core/indexes/range.py

+         .. deprecated:: 0.25.0
+            Use ._range.start or .start instead.
+        """
+        return self._range.start


Unfortunately, fast_parquet uses _start etc. which I hadn't predicted, when I started this PR. So this proposes deprecating the attributes _start, _stop and _step, and removing them in 1.0.

This can be bit of a nuisance for downstream maintainers, so don't know if there's opposition to this, but this PR has several benefits as mentioned, so there's a trade-off.

I think pyarrow uses them as well. IIRC recently had a discussion that they're semi-officially public for advanced users (do you recall that @jorisvandenbossche)

don't recommend ._range.start, just say .start

I think its ok to deprecate these, the downstream will have to adjust

codecov · 2019-05-30T21:21:06Z

Codecov Report

Merging #26581 into master will decrease coverage by 50.17%.
The diff coverage is 40%.

@@             Coverage Diff             @@
##           master   #26581       +/-   ##
===========================================
- Coverage   91.84%   41.67%   -50.18%     
===========================================
  Files         174      174               
  Lines       50644    50625       -19     
===========================================
- Hits        46516    21099    -25417     
- Misses       4128    29526    +25398

Flag	Coverage Δ
#multiple	`?`
#single	`41.67% <40%> (-0.13%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/packers.py	`14.53% <ø> (-73.55%)`	⬇️
pandas/core/series.py	`45.31% <ø> (-48.31%)`	⬇️
pandas/core/frame.py	`34.99% <ø> (-62.13%)`	⬇️
pandas/core/indexes/range.py	`49.84% <35.38%> (-48.13%)`	⬇️
pandas/core/dtypes/concat.py	`52.19% <70%> (-44.38%)`	⬇️
pandas/io/formats/latex.py	`0% <0%> (-100%)`	⬇️
pandas/io/sas/sas_constants.py	`0% <0%> (-100%)`	⬇️
pandas/core/groupby/categorical.py	`0% <0%> (-100%)`	⬇️
pandas/tseries/plotting.py	`0% <0%> (-100%)`	⬇️
pandas/tseries/converter.py	`0% <0%> (-100%)`	⬇️
... and 133 more

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 4c54dd2...d947faa. Read the comment docs.

codecov · 2019-05-30T21:21:17Z

Codecov Report

Merging #26581 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #26581      +/-   ##
==========================================
- Coverage   91.87%   91.87%   -0.01%     
==========================================
  Files         174      174              
  Lines       50692    50672      -20     
==========================================
- Hits        46575    46554      -21     
- Misses       4117     4118       +1

Flag	Coverage Δ
#multiple	`90.41% <100%> (ø)`	⬆️
#single	`41.78% <42.99%> (-0.1%)`	⬇️

Impacted Files	Coverage Δ
pandas/io/packers.py	`88.08% <ø> (ø)`	⬆️
pandas/core/series.py	`93.61% <ø> (ø)`	⬆️
pandas/core/frame.py	`97% <ø> (-0.12%)`	⬇️
pandas/core/dtypes/concat.py	`96.58% <100%> (+0.01%)`	⬆️
pandas/core/dtypes/common.py	`96% <100%> (+0.14%)`	⬆️
pandas/core/indexes/range.py	`98.47% <100%> (+0.42%)`	⬆️
pandas/io/gbq.py	`78.94% <0%> (-10.53%)`	⬇️
pandas/util/testing.py	`90.91% <0%> (+0.1%)`	⬆️

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 454b8c5...228b560. Read the comment docs.

jreback

can you add a test for the deprecations?

jreback · 2019-06-01T14:24:07Z

pandas/core/indexes/range.py

+         .. deprecated:: 0.25.0
+            Use ._range.start or .start instead.
+        """
+        return self._range.start


don't recommend ._range.start, just say .start

jreback · 2019-06-01T14:24:26Z

pandas/core/indexes/range.py

+         .. deprecated:: 0.25.0
+            Use ._range.start or .start instead.
+        """
+        return self._range.start


I think its ok to deprecate these, the downstream will have to adjust

jreback · 2019-06-01T14:24:44Z

pandas/core/indexes/range.py

+        The value of the `stop` parameter
+
+         .. deprecated:: 0.25.0
+            Use ._range.stop or .stop instead.


same as above (you don't need to put the issue numbers in the code)

jreback · 2019-06-01T14:24:52Z

pandas/core/indexes/range.py

+        The value of the `step` parameter (or ``1`` if this was not supplied)
+
+         .. deprecated:: 0.25.0
+            Use ._range.step or .step instead.


jreback · 2019-06-01T14:25:48Z

pandas/core/indexes/range.py

-
-            return RangeIndex._simple_new(start, stop, step, name=self.name)
+            new_range = self._range[key]
+            return RangeIndex.from_range(new_range, name=self.name)


are you using cls everywhere? (or self.class)? (can be either just be consistent)

I wasn't, but I've changed it. For constructor method like this I've done self._from_range(...).

jreback · 2019-06-01T14:26:57Z

also a note in the whatsnew for the deprecation

pep8speaks · 2019-06-01T16:51:43Z

Hello @topper-123! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-06-04 09:18:53 UTC

jreback · 2019-06-02T23:23:04Z

lgtm. @TomAugspurger @jorisvandenbossche any objections?

jreback · 2019-06-02T23:32:57Z

Do we even need to deprecate these or can we just remove? They technically were private before so don't want to set a precedent that have to deprecate internal code going forward as well if we don't have to

@WillAyd see comments above, they are using in pyarrow & fastparquet use them. So best to deprecate.

WillAyd · 2019-06-02T23:35:47Z

Whoops sorry - thanks for clarifying

TomAugspurger · 2019-06-03T02:22:57Z

What's the plan on warning? Should we throw a (not visible) DeprecationWarning for now, and promote that to a FutureWarning? Or just add a FutureWarning after pyarrow / fastparquet can update?

topper-123 · 2019-06-03T03:56:11Z

My plan is to just post an issue on the issue lists for pyarrow & fastparquet with a notice that the attributes will be removed in 1.0 (unless they want us to hold on longer). I think that is sufficient as these are not publicly advertised.

EDIT: My cercern for adding a code message was that it would be visible and therefore annoying for users of those packages. I could add a DeprecationWarning, if that one is more quiet than the usual FutureWarning.

jreback · 2019-06-03T04:13:07Z

ok let’s do a DeprecationWarning now
and then post issues should be good

topper-123 · 2019-06-03T04:49:00Z

I actually see now that DeprecationWarnings are also displayed by default. I thought they were only shown when starting Python with -Wall or similar. Am I missing something?

Uploading anyway now I've done it and there's a demand for the warnings.

jorisvandenbossche · 2019-06-03T05:41:31Z

I actually see now that DeprecationWarnings are also displayed by default

Only if you directly use the deprecated method or attribute, but they should not show up if you eg call a function that uses them (which should be the case for libraries such as pyarrow / fastparquet).

jorisvandenbossche · 2019-06-03T05:45:03Z

Personally, I am -1 on deprecating them (pyarrow/fastparquet only just started to use them, we literally said them it was fine, it's more annoying for them to deal with an attribute name change than for us to just keep it a couple of releases more). But if we're using a deprecation instead of future warning, I won't object if the majority is fine with deprecating.

jreback · 2019-06-03T12:01:23Z

Personally, I am -1 on deprecating them (pyarrow/fastparquet only just started to use them, we literally said them it was fine, it's more annoying for them to deal with an attribute name change than for us to just keep it a couple of releases more). But if we're using a deprecation instead of future warning, I won't object if the majority is fine with deprecating.

right this is the purpose of the DeprecationWarning (rather than FutureWarning) as it won't show-up too much; but the deprecation should happen.

jreback · 2019-06-03T12:02:01Z

@topper-123 can you file an issue on fastparquet and a JIRA on pyarrow.

topper-123 · 2019-06-03T15:44:02Z

No problem. I'd like to have this merged before I write the issues, so they can see the issue by just pulling pandas master.

topper-123 · 2019-06-04T08:47:43Z

Just added a small clean-up, so __new__ won't have a custom & internal ensure_int, but uses the same ensure_python_int as proposed in #26617. This simplifies the code a bit further.

If this gets accepted before #26617, I'll rebase that and add the requested type hints.

jreback · 2019-06-05T12:50:38Z

thanks @topper-123

topper-123 added Clean Index Related to the Index class or subclasses labels May 30, 2019

topper-123 added this to the 0.25.0 milestone May 30, 2019

topper-123 commented May 30, 2019

View reviewed changes

topper-123 force-pushed the clean_RangeIndex branch from 6763b22 to d947faa Compare May 30, 2019 21:20

topper-123 force-pushed the clean_RangeIndex branch 5 times, most recently from 8a0750c to 93d281d Compare May 31, 2019 07:13

jreback requested changes Jun 1, 2019

View reviewed changes

jsexauer mentioned this pull request Jun 1, 2019

DEPR: Clean up list of deprecations from prior versions #6581

Closed

1 task

topper-123 force-pushed the clean_RangeIndex branch from 93d281d to 123013c Compare June 1, 2019 16:51

topper-123 force-pushed the clean_RangeIndex branch 13 times, most recently from dee54ed to f14384c Compare June 2, 2019 13:48

jreback approved these changes Jun 2, 2019

View reviewed changes

topper-123 mentioned this pull request Jun 3, 2019

PERF: custom ops for RangeIndex.[all|any|__contains__] #26617

Merged

4 tasks

topper-123 added 3 commits June 3, 2019 06:18

Use _range instead of _start etc. in RangeIndex

c25e2a4

Changes.

a86cd9a

add deprecation code

233aad0

topper-123 force-pushed the clean_RangeIndex branch from f14384c to 233aad0 Compare June 3, 2019 04:49

topper-123 added Refactor Internal refactoring of code and removed Clean labels Jun 3, 2019

collect ensure_int into ensure_python_int

228b560

topper-123 force-pushed the clean_RangeIndex branch from 98d1881 to 228b560 Compare June 4, 2019 09:18

jreback merged commit 6ce7fc7 into pandas-dev:master Jun 5, 2019

topper-123 deleted the clean_RangeIndex branch June 5, 2019 12:59

topper-123 mentioned this pull request Jun 5, 2019

Pandas 025 has made RangeIndex start, stop & step public and dropped the like-named internal atiributes dask/fastparquet#436

Closed

vaibhavhrt pushed a commit to vaibhavhrt/pandas that referenced this pull request Jun 6, 2019

use range in RangeIndex instead of _start etc. (pandas-dev#26581)

5abb8c3

tseppelt mentioned this pull request May 7, 2020

Deprecated RangeIndex #762 rstudio/reticulate#763

Closed

asfimport mentioned this pull request Jun 17, 2019

[Python] pandas.RangeIndex._start/_stop/_step are deprecated apache/arrow#22046

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

use range in RangeIndex instead of _start etc. #26581

use range in RangeIndex instead of _start etc. #26581

topper-123 commented May 30, 2019

topper-123 May 30, 2019 •

edited

Loading

TomAugspurger May 30, 2019

jreback Jun 1, 2019

jreback Jun 1, 2019

codecov bot commented May 30, 2019

codecov bot commented May 30, 2019 •

edited

Loading

jreback left a comment

jreback Jun 1, 2019

jreback Jun 1, 2019

jreback Jun 1, 2019

jreback Jun 1, 2019

jreback Jun 1, 2019

topper-123 Jun 1, 2019

jreback commented Jun 1, 2019

pep8speaks commented Jun 1, 2019 •

edited

Loading

jreback commented Jun 2, 2019

jreback commented Jun 2, 2019

WillAyd commented Jun 2, 2019

TomAugspurger commented Jun 3, 2019

topper-123 commented Jun 3, 2019 •

edited

Loading

jreback commented Jun 3, 2019

topper-123 commented Jun 3, 2019

jorisvandenbossche commented Jun 3, 2019

jorisvandenbossche commented Jun 3, 2019

jreback commented Jun 3, 2019

jreback commented Jun 3, 2019

topper-123 commented Jun 3, 2019 •

edited

Loading

topper-123 commented Jun 4, 2019

jreback commented Jun 5, 2019

use range in RangeIndex instead of _start etc. #26581

use range in RangeIndex instead of _start etc. #26581

Conversation

topper-123 commented May 30, 2019

topper-123 May 30, 2019 • edited Loading

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

codecov bot commented May 30, 2019

Codecov Report

codecov bot commented May 30, 2019 • edited Loading

Codecov Report

jreback 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

jreback commented Jun 1, 2019

pep8speaks commented Jun 1, 2019 • edited Loading

Comment last updated at 2019-06-04 09:18:53 UTC

jreback commented Jun 2, 2019

jreback commented Jun 2, 2019

WillAyd commented Jun 2, 2019

TomAugspurger commented Jun 3, 2019

topper-123 commented Jun 3, 2019 • edited Loading

jreback commented Jun 3, 2019

topper-123 commented Jun 3, 2019

jorisvandenbossche commented Jun 3, 2019

jorisvandenbossche commented Jun 3, 2019

jreback commented Jun 3, 2019

jreback commented Jun 3, 2019

topper-123 commented Jun 3, 2019 • edited Loading

topper-123 commented Jun 4, 2019

jreback commented Jun 5, 2019

topper-123 May 30, 2019 •

edited

Loading

codecov bot commented May 30, 2019 •

edited

Loading

pep8speaks commented Jun 1, 2019 •

edited

Loading

topper-123 commented Jun 3, 2019 •

edited

Loading

topper-123 commented Jun 3, 2019 •

edited

Loading