REGR: fix bug in DatetimeIndex slicing with irregular or unsorted indices #37023

CloseChoice · 2020-10-10T06:30:19Z

closes #36953
closes #35509

tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

pep8speaks · 2020-10-10T06:30:24Z

Hello @CloseChoice! 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 2020-10-26 12:55:55 UTC

arw2019

Thanks @CloseChoice for the PR!

Some comments.

Can you add the OP from #35509? As per #36953 (comment) the two issues are caused by the same underlying bug but different behaviors so we'd like to have tests for both

I'm also not sure about the current location for this. Haven't looked in detail but somewhere like pandas/tests/frame/test_timeseries.py might be more appropriate

pandas/core/generic.py

pandas/tests/generic/test_generic.py

arw2019 · 2020-10-10T06:38:45Z

pandas/tests/generic/test_generic.py

+
+    def test_slice_incomplete_datetimes(self):
+        # GH36953 and GH35509
+        increment = pd.Series(


I know this is the OP but is it possible to explicitly construct it? without using the cumsum

pandas/tests/generic/test_generic.py

CloseChoice · 2020-10-10T07:38:13Z

Thanks @CloseChoice for the PR!

Some comments.

Can you add the OP from #35509? As per #36953 (comment) the two issues are caused by the same underlying bug but different behaviors so we'd like to have tests for both

I'm also not sure about the current location for this. Haven't looked in detail but somewhere like pandas/tests/frame/test_timeseries.py might be more appropriate

Hi @arw2019 thanks for the quick review. I added another test corresponding to #35509 and moved both tests to your suggested destination pandas/tests/frame/test_timeseries.py.

simonjayhawkins

Thanks @CloseChoice can you add a release note to doc/source/whatsnew/v1.1.4.rst

jreback · 2020-10-10T16:36:40Z

pandas/tests/frame/test_timeseries.py

@@ -63,3 +63,22 @@ def test_datetime_assignment_with_NaT_and_diff_time_units(self):
            {0: [1, None], "new": [1e9, None]}, dtype="datetime64[ns]"
        )
        tm.assert_frame_equal(result, expected)
+
+    def test_slice_irregular_datetime_index_with_nan(self):


move these to pandas/tests/indexing/test_partial.py

always use
result=
expected=
tm.assert_.....

jreback · 2020-10-10T16:37:53Z

also this needs a whatsnew note for 1.1.4 in regressions (mention both issues)

…G-regression-datetimeindex-slicing

mroeschke · 2020-10-11T21:01:32Z

pandas/tests/indexing/test_partial.py

+        index = pd.to_datetime(["2012-01-01", "2012-01-02", "2012-01-03", None])
+        df = pd.DataFrame(range(len(index)), index=index)
+        expected = pd.DataFrame(range(len(index[:3])), index=index[:3])
+        tm.assert_frame_equal(df["2012-01-01":"2012-01-04"], expected)


Could you define result = df["2012-01-01":"2012-01-04"] above?

mroeschke · 2020-10-11T21:01:46Z

pandas/tests/indexing/test_partial.py

+            {"col1": ["a", "c"], "col2": [1, 3]},
+            index=pd.to_datetime(["2020-08-01", "2020-08-05"]),
+        )
+        tm.assert_frame_equal(df.loc["2020-08"], expected)


Could you define result = df["2020-08"] above?

jreback · 2020-10-11T21:28:54Z

pandas/core/generic.py

@@ -3739,7 +3739,6 @@ def _slice(self: FrameOrSeries, slobj: slice, axis=0) -> FrameOrSeries:

        Slicing with this method is *always* positional.
        """
-        assert isinstance(slobj, slice), type(slobj)


this is not the patch; something is going down the wrong path and ending up here. this routine is only for slices.

Ok, I will look into that. I just wanted to note that in version 1.0.9 it went down just the same path except that the exception wasn't there yet. In this sense we are not talking about a regression but about fixing a bug which seems to have been there since a while ago.

@CloseChoice can you check if the suggestion from here #35509 (comment) passes the tests you added?

…G-regression-datetimeindex-slicing

…tetimeindex-slicing

simonjayhawkins · 2020-10-26T11:40:18Z

@CloseChoice can you fix the pre-commit errors. (I'm on Windows and pre-commit fails to initialize and these checks can no longer be performed locally from code-checks.sh #37240) cc @MarcoGorelli

MarcoGorelli · 2020-10-26T12:52:52Z

@CloseChoice can you fix the pre-commit errors. (I'm on Windows and pre-commit fails to initialize and these checks can no longer be performed locally from code-checks.sh #37240) cc @MarcoGorelli

@CloseChoice you just need to replace pd.DataFrame with DataFrame

@simonjayhawkins what errors do you get? Part of the motivation for moving these checks over to pre-commit was that they'd be cross-platform

EDIT

@simonjayhawkins the pre-commit hooks all pass, it's the code-checks.sh that fail 🤣

pandas/tests/indexing/test_partial.py

simonjayhawkins · 2020-10-26T13:09:01Z

@simonjayhawkins what errors do you get?
i'm using a different machine, so have not looked in detail yet. (and can't remember what did last time)

version information

pre-commit version: 2.7.1
sys.version:
    3.8.6 | packaged by conda-forge | (default, Oct  7 2020, 18:22:52) [MSC v.1916 64 bit (AMD64)]
sys.executable: C:\Users\simon\anaconda3\envs\pandas-dev\python.exe
os.name: nt
sys.platform: win32

error information

An unexpected error has occurred: CalledProcessError: command: ('C:\\Users\\simon\\anaconda3\\envs\\pandas-dev\\python.exe', '-mvirtualenv', 'C:\\Users\\simon\\.cache\\pre-commit\\repoyew02fci\\py_env-python3', '-p', 'C:\\Users\\simon\\anaconda3\\envs\\pandas-dev\\python.exe')
return code: 1
expected return code: 0
stdout:
    FileNotFoundError: [Errno 2] No such file or directory: 'C:\\Users\\simon\\anaconda3\\envs\\pandas-dev\\Lib\\venv\\scripts\\nt\\python.exe'
    
stderr: (none)

Traceback (most recent call last):
  File "C:\Users\simon\anaconda3\envs\pandas-dev\lib\site-packages\pre_commit\error_handler.py", line 63, in error_handler
    yield
  File "C:\Users\simon\anaconda3\envs\pandas-dev\lib\site-packages\pre_commit\main.py", line 390, in main
    return run(args.config, store, args)
  File "C:\Users\simon\anaconda3\envs\pandas-dev\lib\site-packages\pre_commit\commands\run.py", line 388, in run
    install_hook_envs(hooks, store)
  File "C:\Users\simon\anaconda3\envs\pandas-dev\lib\site-packages\pre_commit\repository.py", line 206, in install_hook_envs
    _hook_install(hook)
  File "C:\Users\simon\anaconda3\envs\pandas-dev\lib\site-packages\pre_commit\repository.py", line 82, in _hook_install
    lang.install_environment(
  File "C:\Users\simon\anaconda3\envs\pandas-dev\lib\site-packages\pre_commit\languages\python.py", line 213, in install_environment
    cmd_output_b(*venv_cmd, cwd='/')
  File "C:\Users\simon\anaconda3\envs\pandas-dev\lib\site-packages\pre_commit\util.py", line 157, in cmd_output_b
    raise CalledProcessError(returncode, cmd, retcode, stdout_b, stderr_b)
pre_commit.util.CalledProcessError: command: ('C:\\Users\\simon\\anaconda3\\envs\\pandas-dev\\python.exe', '-mvirtualenv', 'C:\\Users\\simon\\.cache\\pre-commit\\repoyew02fci\\py_env-python3', '-p', 'C:\\Users\\simon\\anaconda3\\envs\\pandas-dev\\python.exe')
return code: 1
expected return code: 0
stdout:
    FileNotFoundError: [Errno 2] No such file or directory: 'C:\\Users\\simon\\anaconda3\\envs\\pandas-dev\\Lib\\venv\\scripts\\nt\\python.exe'
    
stderr: (none)

Part of the motivation for moving these checks over to pre-commit was that they'd be cross-platform

I understand that, but didn't we discuss that pre-commit was optional.

@simonjayhawkins the pre-commit hooks all pass, it's the code-checks.sh that fail 🤣

my bad. missed that.

MarcoGorelli · 2020-10-26T14:15:03Z

@simonjayhawkins thanks for the traceback - are you using conda? I could reproduce that on a windows conda environment. Looks like it's a bug in virtualenv pypa/virtualenv#1986

If you downgrade virtualenv to 20.0.33 then it should work (did for me) - if it works for you I'll note it in the docs

simonjayhawkins · 2020-10-26T14:40:48Z

If you downgrade virtualenv to 20.0.33 then it should work (did for me) - if it works for you I'll note it in the docs

thanks works for me too. (20.0.35 was installed). just started running on all-files. will report back if fails.

simonjayhawkins · 2020-10-26T14:47:54Z

back to the issue

so pls merge master and let's see what the CI says

failures on Windows py38_np18 seen on other PRs.

jreback · 2020-10-26T14:52:02Z

thanks @CloseChoice

…with irregular or unsorted indices

MarcoGorelli · 2020-10-26T15:05:10Z

will report back if fails.

although it installed fine for me, some hooks still failed, likely because currently we have

./scripts/validate_unwanted_patterns --validation-type="private_function_across_module"

instead of

python scripts/validate_unwanted_patterns --validation-type="private_function_across_module"

, will fix

simonjayhawkins · 2020-10-26T15:15:06Z

will report back if fails.

although it installed fine for me, some hooks still failed, likely because currently we have

works for me in anaconda prompt and git bash

…ular or unsorted indices (#37418) Co-authored-by: Tobias Pitters <31857876+CloseChoice@users.noreply.github.com>

…ices (pandas-dev#37023)

remove assert in core/generic.py and add test for datetimeindex slicing

f1d4a10

CloseChoice added 2 commits October 10, 2020 08:31

fix formatting in test_generic

cd1a136

fix test and remove commented line

fe478c4

arw2019 suggested changes Oct 10, 2020

View reviewed changes

updated tests according to PR comments

15a9185

CloseChoice added 3 commits October 10, 2020 10:10

slice with loc to prevent future warning

6caba47

revert changes in io/formats/css.py

5703994

run black on test_timeseries.py

4a7ff03

simonjayhawkins added Indexing Related to indexing on series/frames, not to indexes themselves Regression Functionality that used to work in a prior pandas version Datetime Datetime data dtype labels Oct 10, 2020

simonjayhawkins added this to the 1.1.4 milestone Oct 10, 2020

simonjayhawkins requested changes Oct 10, 2020

View reviewed changes

arw2019 approved these changes Oct 10, 2020

View reviewed changes

jreback requested changes Oct 10, 2020

View reviewed changes

CloseChoice added 4 commits October 11, 2020 19:57

move tests to indexing/test_partial, add whatsnew entry

25b229c

reformat test_timeseries.py

9bb10fe

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

9d82f83

…G-regression-datetimeindex-slicing

update whatsnew

968fdf4

mroeschke reviewed Oct 11, 2020

View reviewed changes

jreback requested changes Oct 11, 2020

View reviewed changes

jorisvandenbossche changed the title ~~remove assert in core/generic.py and add test for datetimeindex slicing~~ REGR: fix bug in DatetimeIndex slicing with irregular or unsorted indices Oct 12, 2020

CloseChoice added 4 commits October 16, 2020 20:27

Merge branch 'master' into BUG-regression-datetimeindex-slicing

89b0248

add suggestion from comment

3710514

use result and expected in tests

237b085

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

43a2a26

…G-regression-datetimeindex-slicing

CloseChoice and others added 2 commits October 22, 2020 19:30

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

4261f02

…G-regression-datetimeindex-slicing

Merge remote-tracking branch 'upstream/master' into BUG-regression-da…

583003e

…tetimeindex-slicing

simonjayhawkins mentioned this pull request Oct 26, 2020

DISC: Use of assertions in user-facing code #37350

Closed

simonjayhawkins mentioned this pull request Oct 26, 2020

RLS: 1.1.4 #37397

Closed

MarcoGorelli reviewed Oct 26, 2020

View reviewed changes

pandas/tests/indexing/test_partial.py Outdated Show resolved Hide resolved

MarcoGorelli reviewed Oct 26, 2020

View reviewed changes

pandas/tests/indexing/test_partial.py Outdated Show resolved Hide resolved

MarcoGorelli reviewed Oct 26, 2020

View reviewed changes

pandas/tests/indexing/test_partial.py Outdated Show resolved Hide resolved

MarcoGorelli reviewed Oct 26, 2020

View reviewed changes

pandas/tests/indexing/test_partial.py Outdated Show resolved Hide resolved

MarcoGorelli added 4 commits October 26, 2020 12:55

Update pandas/tests/indexing/test_partial.py

e246520

Update pandas/tests/indexing/test_partial.py

9cf919e

Update pandas/tests/indexing/test_partial.py

7f693dc

Update pandas/tests/indexing/test_partial.py

02ae223

jreback approved these changes Oct 26, 2020

View reviewed changes

jreback merged commit 425ce1b into pandas-dev:master Oct 26, 2020

meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Oct 26, 2020

Backport PR pandas-dev#37023: REGR: fix bug in DatetimeIndex slicing …

ee61ca6

…with irregular or unsorted indices

meeseeksmachine mentioned this pull request Oct 26, 2020

Backport PR #37023 on branch 1.1.x (REGR: fix bug in DatetimeIndex slicing with irregular or unsorted indices) #37418

Merged

MarcoGorelli mentioned this pull request Oct 26, 2020

fix windows issue pre-commit #37419

Merged

simonjayhawkins pushed a commit that referenced this pull request Oct 26, 2020

Backport PR #37023: REGR: fix bug in DatetimeIndex slicing with irreg…

df22136

…ular or unsorted indices (#37418) Co-authored-by: Tobias Pitters <31857876+CloseChoice@users.noreply.github.com>

MarcoGorelli mentioned this pull request Oct 31, 2020

DOC Add note about virtualenv and conda #37542

Merged

kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020

REGR: fix bug in DatetimeIndex slicing with irregular or unsorted ind…

c44537b

…ices (pandas-dev#37023)

bast0006 mentioned this pull request Jan 11, 2021

ExtensionBlock.take_nd crashes in 1.1.0 #35768

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

REGR: fix bug in DatetimeIndex slicing with irregular or unsorted indices #37023

REGR: fix bug in DatetimeIndex slicing with irregular or unsorted indices #37023

CloseChoice commented Oct 10, 2020 •

edited by jreback

Loading

pep8speaks commented Oct 10, 2020 •

edited

Loading

arw2019 left a comment

arw2019 Oct 10, 2020

CloseChoice Oct 10, 2020

CloseChoice commented Oct 10, 2020

simonjayhawkins left a comment

jreback Oct 10, 2020

jreback commented Oct 10, 2020

mroeschke Oct 11, 2020

mroeschke Oct 11, 2020

jreback Oct 11, 2020

CloseChoice Oct 12, 2020

jorisvandenbossche Oct 12, 2020

simonjayhawkins commented Oct 26, 2020

MarcoGorelli commented Oct 26, 2020 •

edited

Loading

simonjayhawkins commented Oct 26, 2020

version information

error information

MarcoGorelli commented Oct 26, 2020

simonjayhawkins commented Oct 26, 2020

simonjayhawkins commented Oct 26, 2020

jreback commented Oct 26, 2020

MarcoGorelli commented Oct 26, 2020 •

edited

Loading

simonjayhawkins commented Oct 26, 2020

REGR: fix bug in DatetimeIndex slicing with irregular or unsorted indices #37023

REGR: fix bug in DatetimeIndex slicing with irregular or unsorted indices #37023

Conversation

CloseChoice commented Oct 10, 2020 • edited by jreback Loading

pep8speaks commented Oct 10, 2020 • edited Loading

Comment last updated at 2020-10-26 12:55:55 UTC

arw2019 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CloseChoice commented Oct 10, 2020

simonjayhawkins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Oct 10, 2020

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

simonjayhawkins commented Oct 26, 2020

MarcoGorelli commented Oct 26, 2020 • edited Loading

EDIT

simonjayhawkins commented Oct 26, 2020

version information

error information

MarcoGorelli commented Oct 26, 2020

simonjayhawkins commented Oct 26, 2020

simonjayhawkins commented Oct 26, 2020

jreback commented Oct 26, 2020

MarcoGorelli commented Oct 26, 2020 • edited Loading

simonjayhawkins commented Oct 26, 2020

CloseChoice commented Oct 10, 2020 •

edited by jreback

Loading

pep8speaks commented Oct 10, 2020 •

edited

Loading

MarcoGorelli commented Oct 26, 2020 •

edited

Loading

MarcoGorelli commented Oct 26, 2020 •

edited

Loading