issue #27642 - timedelta merge asof with tolerance #27650

ianzur · 2019-07-29T21:35:20Z

closes merge_asof(): cannot use tolerance flag when the index is a TimedeltaIndex #27642
tests 2 / 1 [added / passed]
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry
(Optional) allow users to pass datetime.timedelta objects for tolerance

pep8speaks · 2019-07-29T21:35:23Z

Hello @ianzur! 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-08-22 11:36:23 UTC

ianzur · 2019-07-29T21:40:03Z

My first PR, please be gentle.

TomAugspurger

Can you add a release note to doc/source/whatsnew/v0.25.1.rst?

We'll also want a test ensuring this continues to work in the future. Can you simplify your example from #27642 and make it a unit test? Make sure you include a reference to the GitHub issue in a comment in the test.

pandas/core/reshape/merge.py

mroeschke · 2019-07-29T22:04:44Z

pandas/core/reshape/merge.py

+                is_datetime64_dtype(lt)
+                or is_datetime64tz_dtype(lt)
+                or is_timedelta64_dtype(lt)
+            ):
                if not isinstance(self.tolerance, Timedelta):


This should probably be if not isinstance(self.tolerance, datetime.timedelta) so users can pass datetime.timedelta objects. If you could add a test case for that situation that'd be great.

see commit "mroeschke: this test is for you 🚀"

the test fails

I think we'll need a different PR for this test I think, it fails at this conversion check.

pandas/pandas/core/reshape/merge.py

Lines 1704 to 1710 in 143bc34

# initial type conversion as needed

if needs_i8_conversion(left_values):

left_values = left_values.view("i8")

right_values = right_values.view("i8")

if tolerance is not None:

tolerance = tolerance.value

With tolerance = datetime.timedelta -> ERROR: has no attribute "value"

datetime.timedelta objects do not have nanoseconds like pd.Timedelta

Another check and conversion is necessary to convert datetime.timedelta correctly to nanoseconds similarly to pd.Timedelta.value

Okay thanks for investigating. Yes, I'd be great to address in another issues/PR.

doc/source/whatsnew/v0.25.1.rst

pandas/tests/reshape/merge/test_merge_asof.py

…delta-merge-asof

ianzur · 2019-08-08T15:27:01Z

Does anyone understand why my test fails in python3.5?

WillAyd · 2019-08-08T15:57:30Z

On phone so haven’t checked logs but if 3.5 only most likely the construction of data frames from Dict don’t have a defined order. Probably easiest to construct from lists

…

Sent from my iPhone

On Aug 8, 2019, at 8:27 AM, ianzur ***@***.***> wrote: Does anyone understand why my test fails in python3.5? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

…delta-merge-asof

TomAugspurger · 2019-08-12T19:03:59Z

@ianzur you may want to set up a 3.5 env locally for testing that. conda env create -n pandas-azure-35-compat -f ci/deps/azure-35-compat.yaml

…-timedelta-merge-asof

TomAugspurger · 2019-08-22T13:08:45Z

pandas/tests/reshape/merge/test_merge_asof.py

+            Timedelta("1day"),
+            pytest.param(
+                datetime.timedelta(days=1),
+                marks=pytest.mark.xfail(reason="not implemented", strict=True),


@ianzur Do we have an open issue to implement this?

No, not that I know of.

…e asof with tolerance

TomAugspurger · 2019-08-22T14:02:16Z

Could you open one with an example of the failure right now?

…

On Thu, Aug 22, 2019 at 8:27 AM ianzur ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In pandas/tests/reshape/merge/test_merge_asof.py <#27650 (comment)>: > @@ -588,14 +590,23 @@ def test_non_sorted(self): # ok, though has dupes merge_asof(trades, self.quotes, on="time", by="ticker") - def test_tolerance(self): + @pytest.mark.parametrize( + "tolerance", + [ + Timedelta("1day"), + pytest.param( + datetime.timedelta(days=1), + marks=pytest.mark.xfail(reason="not implemented", strict=True), No, not that I know of. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#27650?email_source=notifications&email_token=AAKAOIU5BQ66CXKQS367NEDQF2H55A5CNFSM4IHWN4YKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCCME7GY#discussion_r316676824>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKAOIR2IAWBEU2N2CDI5TTQF2H55ANCNFSM4IHWN4YA> .

#28088)

ianzur · 2019-08-22T15:34:18Z

I can later today, I'll @you. On Thu, Aug 22, 2019 at 9:03 AM Tom Augspurger <notifications@github.com> wrote:

…

Could you open one with an example of the failure right now? On Thu, Aug 22, 2019 at 8:27 AM ianzur ***@***.***> wrote: > ***@***.**** commented on this pull request. > ------------------------------ > > In pandas/tests/reshape/merge/test_merge_asof.py > <#27650 (comment)>: > > > @@ -588,14 +590,23 @@ def test_non_sorted(self): > # ok, though has dupes > merge_asof(trades, self.quotes, on="time", by="ticker") > > - def test_tolerance(self): > + @pytest.mark.parametrize( > + "tolerance", > + [ > + Timedelta("1day"), > + pytest.param( > + datetime.timedelta(days=1), > + marks=pytest.mark.xfail(reason="not implemented", strict=True), > > No, not that I know of. > > — > You are receiving this because you modified the open/close state. > Reply to this email directly, view it on GitHub > < #27650?email_source=notifications&email_token=AAKAOIU5BQ66CXKQS367NEDQF2H55A5CNFSM4IHWN4YKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCCME7GY#discussion_r316676824 >, > or mute the thread > < https://github.com/notifications/unsubscribe-auth/AAKAOIR2IAWBEU2N2CDI5TTQF2H55ANCNFSM4IHWN4YA > > . > — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27650?email_source=notifications&email_token=AICYMWLKCMZOYWV7C5F4QBTQF2MCFA5CNFSM4IHWN4YKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD45GEOI#issuecomment-523919929>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AICYMWOVBXYT7QB5BPLVE6DQF2MCFANCNFSM4IHWN4YA> .

* issue pandas-dev#27642 - timedelta merge asof with tolerance

* master: (40 commits) DOC: Fix GL01 and GL02 errors in the docstrings (pandas-dev#27988) Remove Encoding of values in char** For Labels (pandas-dev#27618) TYPING: more type hints for io.formats.printing (pandas-dev#27765) TST: fix compression tests when run without virtualenv/condaenv (pandas-dev#28051) DOC: Start 0.25.2 (pandas-dev#28111) DOC: Fix docstrings lack of punctuation (pandas-dev#28031) DOC: Remove alias for numpy.random.randn from the docs (pandas-dev#28082) DOC: update GroupBy.head()/tail() documentation (pandas-dev#27844) BUG: timedelta merge asof with tolerance (pandas-dev#27650) BUG: Series.rename raises error on values accepted by Series construc… (pandas-dev#27814) Preserve index when setting new column on empty dataframe. (pandas-dev#26471) BUG: Fixed groupby quantile for listlike q (pandas-dev#27827) BUG: iter with readonly values, closes pandas-dev#28055 (pandas-dev#28074) TST: non-strict xfail for period test (pandas-dev#28072) DOC: Update whatsnew (pandas-dev#28073) CI: disable codecov (pandas-dev#28065) CI: Set SHA for codecov upload (pandas-dev#28067) BUG: Correct the previous bug fixing on xlim for plotting (pandas-dev#28059) CI: Add pip dependence explicitly (pandas-dev#28008) DOC: Change document code prun in a row (pandas-dev#28029) ...

issue #27642 - timedelta merge asof with tolerance

533d407

#27642

431da66

TomAugspurger added this to the 0.25.1 milestone Jul 29, 2019

TomAugspurger added Reshaping Concat, Merge/Join, Stack/Unstack, Explode Timedelta Timedelta data type labels Jul 29, 2019

TomAugspurger reviewed Jul 29, 2019

View reviewed changes

pandas/core/reshape/merge.py Outdated Show resolved Hide resolved

mroeschke reviewed Jul 29, 2019

View reviewed changes

ianzur added 6 commits July 30, 2019 00:38

whatsnew

c9acc09

#27642 added testing stuff

76f6525

mroeschke: this test is for you 🚀

fd1dbb2

some words

63b7a2b

remove some whitespace

84759ac

a comment

04c18e9

jreback requested changes Jul 31, 2019

View reviewed changes

doc/source/whatsnew/v0.25.1.rst Outdated Show resolved Hide resolved

pandas/tests/reshape/merge/test_merge_asof.py Outdated Show resolved Hide resolved

ianzur added 9 commits July 31, 2019 13:14

docs change

ac76918

Merge remote-tracking branch 'upstream/master' into issue-#27642-time…

0a0c5c8

…delta-merge-asof

callbacks are 🎆

a26bb71

whitespace

a0ba5d9

little stuff

e5456ac

isort

4f56dc4

I think this'll work

52193a4

cleaner

7c004c9

Merge remote-tracking branch 'upstream/master' into issue-#27642-time…

3f8bf2b

…delta-merge-asof

ianzur added 3 commits August 9, 2019 10:41

🐛 🔧

8f8aeb3

blacked

f66fce4

unused import

0eb173f

ianzur closed this Aug 9, 2019

ianzur reopened this Aug 9, 2019

Merge remote-tracking branch 'upstream/master' into issue-#27642-time…

4a1c515

…delta-merge-asof

ianzur and others added 4 commits August 16, 2019 09:22

resolved merge conflict in whatsnew

6daf823

Merge remote-tracking branch 'upstream/master' into ianzur-issue-#27642…

3688986

…-timedelta-merge-asof

Merge remote-tracking branch 'upstream/master' into ianzur-issue-#27642…

f6009ca

…-timedelta-merge-asof

Removed duplicate

6f7151d

TomAugspurger approved these changes Aug 22, 2019

View reviewed changes

TomAugspurger reviewed Aug 22, 2019

View reviewed changes

TomAugspurger merged commit def01cf into pandas-dev:master Aug 22, 2019

meeseeksmachine pushed a commit to meeseeksmachine/pandas that referenced this pull request Aug 22, 2019

Backport PR pandas-dev#27650: issue pandas-dev#27642 - timedelta merg…

b39ed65

…e asof with tolerance

meeseeksmachine mentioned this pull request Aug 22, 2019

Backport PR #27650 on branch 0.25.x (issue #27642 - timedelta merge asof with tolerance) #28088

Merged

TomAugspurger pushed a commit that referenced this pull request Aug 22, 2019

Backport PR #27650: issue #27642 - timedelta merge asof with tolerance (

7b44c0a

#28088)

galuhsahid pushed a commit to galuhsahid/pandas that referenced this pull request Aug 25, 2019

BUG: timedelta merge asof with tolerance (pandas-dev#27650)

c57f608

* issue pandas-dev#27642 - timedelta merge asof with tolerance

ianzur deleted the issue-#27642-timedelta-merge-asof branch June 27, 2021 18:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

issue #27642 - timedelta merge asof with tolerance #27650

issue #27642 - timedelta merge asof with tolerance #27650

ianzur commented Jul 29, 2019 •

edited

Loading

pep8speaks commented Jul 29, 2019 •

edited

Loading

ianzur commented Jul 29, 2019

TomAugspurger left a comment

mroeschke Jul 29, 2019

ianzur Jul 30, 2019

ianzur Jul 30, 2019

ianzur Jul 30, 2019 •

edited

Loading

ianzur Jul 30, 2019 •

edited

Loading

mroeschke Jul 30, 2019

ianzur commented Aug 8, 2019

WillAyd commented Aug 8, 2019 via email

TomAugspurger commented Aug 12, 2019

TomAugspurger Aug 22, 2019 •

edited

Loading

ianzur Aug 22, 2019

TomAugspurger commented Aug 22, 2019 via email

ianzur commented Aug 22, 2019 via email

	# initial type conversion as needed
	if needs_i8_conversion(left_values):
	left_values = left_values.view("i8")
	right_values = right_values.view("i8")
	if tolerance is not None:
	tolerance = tolerance.value

issue #27642 - timedelta merge asof with tolerance #27650

issue #27642 - timedelta merge asof with tolerance #27650

Conversation

ianzur commented Jul 29, 2019 • edited Loading

pep8speaks commented Jul 29, 2019 • edited Loading

Comment last updated at 2019-08-22 11:36:23 UTC

ianzur commented Jul 29, 2019

TomAugspurger left a comment

Choose a reason for hiding this comment

mroeschke Jul 29, 2019

Choose a reason for hiding this comment

ianzur Jul 30, 2019

Choose a reason for hiding this comment

ianzur Jul 30, 2019

Choose a reason for hiding this comment

ianzur Jul 30, 2019 • edited Loading

Choose a reason for hiding this comment

ianzur Jul 30, 2019 • edited Loading

Choose a reason for hiding this comment

mroeschke Jul 30, 2019

Choose a reason for hiding this comment

ianzur commented Aug 8, 2019

WillAyd commented Aug 8, 2019 via email

TomAugspurger commented Aug 12, 2019

TomAugspurger Aug 22, 2019 • edited Loading

Choose a reason for hiding this comment

ianzur Aug 22, 2019

Choose a reason for hiding this comment

TomAugspurger commented Aug 22, 2019 via email

ianzur commented Aug 22, 2019 via email

ianzur commented Jul 29, 2019 •

edited

Loading

pep8speaks commented Jul 29, 2019 •

edited

Loading

ianzur Jul 30, 2019 •

edited

Loading

ianzur Jul 30, 2019 •

edited

Loading

TomAugspurger Aug 22, 2019 •

edited

Loading