TST: query with timezone aware index & column #34021

vipulr8 · 2020-05-06T11:14:28Z

closes "Cannot compare tz-naive and tz-aware datetime-like objects" when querying DatetimeIndex #29463
tests added / passed
passes black pandas
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

MarcoGorelli · 2020-05-06T13:35:34Z

pandas/tests/frame/indexing/test_column_vs_index_tz.py

+            "Africa/Abidjan",
+            "Africa/Douala",
+            "Africa/Mbabane",
+            "America/Argentina/Catamarca",
+            "America/Belize",
+            "America/Curacao",
+            "America/Guatemala",
+            "America/Kentucky/Louisville",
+            "America/Mexico_City",
+            "America/Port-au-Prince",
+            "America/Sitka",
+            "Antarctica/Casey",
+            "Asia/Ashkhabad",
+            "Asia/Dubai",
+            "Asia/Khandyga",
+            "Asia/Qatar",
+            "Asia/Tomsk",
+            "Atlantic/Reykjavik",
+            "Australia/Queensland",
+            "Canada/Yukon",
+            "Etc/GMT+7",
+            "Etc/UCT",
+            "Europe/Guernsey",
+            "Europe/Paris",
+            "Europe/Vienna",
+            "Indian/Cocos",
+            "NZ",
+            "Pacific/Honolulu",
+            "Pacific/Samoa",
+            "US/Eastern",


Thanks @vipulrai91

There's a fixture you can use here, tz_aware_fixture (feel free to ask for help if it's unclear how to use it)

@MarcoGorelli thank you for the feedback.

As far as I understand the change has to be.

def test_check_column_vs_index_tz_query(self, tz_aware_fixture): tz_list = tz_aware_fixture

Got this snippet from test_datetime.py , but what actually is tz_aware_fixture?
Also , How Do I run the test locally, I tried going through the docs , but will need some help here.

Thanks

tz = tz_aware_fixture should be enough, then pytest will loop through the different timezones defined in tz_aware_fixture

what actually is tz_aware_fixture?

It's a fixture which you can use to test different timezones.

How Do I run the test locally

See using pytest. So here,

$ pytest pandas/tests/frame/indexing/test_column_vs_index_tz.py

should be enough

Thank you for guiding.

tz_aware_fixture is a combination of TimeZones and other object types , also even
it throws pytz.exceptions.UnknownTimeZone error.

test_column_vs_index_tz.py:58: TypeError ============================================================ short test summary info ============================================================= FAILED test_column_vs_index_tz.py::TestColumnvsIndexTZEquality::test_check_column_vs_index_tz_query['UTC'] - pytz.exceptions.UnknownTimeZoneErr... FAILED test_column_vs_index_tz.py::TestColumnvsIndexTZEquality::test_check_column_vs_index_tz_query['US/Eastern'] - pytz.exceptions.UnknownTime... FAILED test_column_vs_index_tz.py::TestColumnvsIndexTZEquality::test_check_column_vs_index_tz_query['Asia/Tokyo'] - pytz.exceptions.UnknownTime... FAILED test_column_vs_index_tz.py::TestColumnvsIndexTZEquality::test_check_column_vs_index_tz_query['dateutil/US/Pacific'] - pytz.exceptions.Un... FAILED test_column_vs_index_tz.py::TestColumnvsIndexTZEquality::test_check_column_vs_index_tz_query['dateutil/Asia/Singapore'] - pytz.exception... FAILED test_column_vs_index_tz.py::TestColumnvsIndexTZEquality::test_check_column_vs_index_tz_query[tzutc()] - TypeError: 'tzutc' object is not... FAILED test_column_vs_index_tz.py::TestColumnvsIndexTZEquality::test_check_column_vs_index_tz_query[tzlocal()] - TypeError: 'tzlocal' object is... FAILED test_column_vs_index_tz.py::TestColumnvsIndexTZEquality::test_check_column_vs_index_tz_query[pytz.FixedOffset(300)] - TypeError: '_Fixed... FAILED test_column_vs_index_tz.py::TestColumnvsIndexTZEquality::test_check_column_vs_index_tz_query[<UTC>] - TypeError: 'UTC' object is not ite... FAILED test_column_vs_index_tz.py::TestColumnvsIndexTZEquality::test_check_column_vs_index_tz_query[pytz.FixedOffset(-300)] - TypeError: '_Fixe... FAILED test_column_vs_index_tz.py::TestColumnvsIndexTZEquality::test_check_column_vs_index_tz_query[datetime.timezone.utc] - TypeError: 'dateti... FAILED test_column_vs_index_tz.py::TestColumnvsIndexTZEquality::test_check_column_vs_index_tz_query[datetime.timezone(datetime.timedelta(seconds=3600))] FAILED test_column_vs_index_tz.py::TestColumnvsIndexTZEquality::test_check_column_vs_index_tz_query[datetime.timezone(datetime.timedelta(days=-1, seconds=82800), 'foo')]

mroeschke · 2020-05-06T16:41:46Z

pandas/tests/frame/indexing/test_column_vs_index_tz.py

+import pandas._testing as tm
+
+
+class TestColumnvsIndexTZEquality:


Don't need an individual class for this. Writing the test as a function is sufficient.

Don't need a new file for just this test. This test can live in pandas/tests/frame/test_query_eval.py

mroeschke · 2020-05-06T16:42:46Z

pandas/tests/frame/indexing/test_column_vs_index_tz.py

+
+class TestColumnvsIndexTZEquality:
+    # https://github.com/pandas-dev/pandas/issues/29463
+    def check_for_various_tz(self, tz):


As mentioned by @MarcoGorelli, you can just use the tz_aware_fixture here

mroeschke · 2020-05-06T16:43:33Z

pandas/tests/frame/indexing/test_column_vs_index_tz.py

+        # # This was earlier raising an exception.
+        index_query = df.set_index("time").query('"2019-01-03 00:00:00+00" < time')
+        l2 = pd.DataFrame(list(index_query.index))
+        tm.assert_frame_equal(l1, l2)


Can you rename the variables so this reads tm.assert_frame_equal(result, expected)

mroeschke · 2020-05-06T16:44:41Z

pandas/tests/frame/indexing/test_column_vs_index_tz.py

+            }
+        )
+        df_query = df.query('"2019-01-03 00:00:00+00" < time')
+        l1 = pd.DataFrame(list(df_query["time"]))


Is this is the "expected" output, we'll want to construct this with a different method without query

expected = pd.DataFrame(...)

@mroeschke ,
I modified everything as per suggestion and the tests pass locally.
But here instead of constructing new "expected" DataFrame we would want to use the ones already generating by using query since "We are testing for result to be same at index and column level"
This is what was expected as per issue ? Right ?

We want to make sure the "expected" DataFrame takes an independent path than query so if query breaks both "result" and "expected" don't change.

So want to make sure the result of:
result = df.set_index('time').query('"2019-01-03 00:00:00+00" < time')

gives a result of:
expected = pd.DataFrame(...)

When we create an expected dataframe , it passes for few tz and fails for others , bcoz the query string always expects the time zone to be "UTC" , query('"2019-01-03 00:00:00+00" < time'), and therefore there is shape mismatch for example for "Asia/Kolkata" it is failing whereas it passes for "US/Eastern".

I assume we cannot change the query string because that is what we are testing for , in that case knowing the expected o/p when comparing to different tz will result in shape mismatch which is exactly happening now.
Any thoughts ?

def test_check_column_vs_index_tz_query(self, tz_aware_fixture): # https://github.com/pandas-dev/pandas/issues/29463 tz = tz_aware_fixture expected = pd.DataFrame( {"time": pd.date_range(start="2019-01-04", freq="1d", periods=7, tz=tz)} ).set_index("time") df = pd.DataFrame( {"time": pd.date_range(start="2019-01-01", freq="1d", periods=10, tz=tz)} ) result = df.set_index("time").query('"2019-01-03 00:00:00+00" < time') tm.assert_frame_equal(result, expected)

AssertionError: DataFrame are different DataFrame shape mismatch [left]: (7, 0) [right]: (8, 0)

Since we're just testing that we can query with a tz offset in the query string when the index is tz-aware, we can be with what the query string is.

You can just change the test to be then

tz = tz_aware_fixture expected = pd.DataFrame(index=pd.date_range(start="2019-01-04", freq="1d", periods=10, tz=tz, name='time')) df = pd.DataFrame(index=pd.date_range(start="2019-01-01", freq="1d", periods=10, tz=tz, name=time)) result = df.query('"2018-01-03 00:00:00+00" < time') tm.assert_frame_equal(result, expected)

@mroeschke , Yes this works , I am now banging my head for not thinking this earlier.
Thank you very much , now all the tests pass.

As suggested , I have added the test in test_query_eval.py file.

MarcoGorelli

Nice one @vipulrai91 , looks better like this, just got a comment about the dates

MarcoGorelli · 2020-05-06T20:24:03Z

pandas/tests/frame/test_query_eval.py

+                start="2019-01-01", freq="1d", periods=10, tz=tz, name="time"
+            )
+        )
+        result = df.query('"2018-01-03 00:00:00+00" < time')


Like this, time is always greater than 2018-01-03 00:00:00+00 - I think @mroeschke was suggesting to use, say, 2019-01-04 in expected, 2019-01-01 in df, and then 2019-01-03 in query

@MarcoGorelli Thank you for the comment / appreciation.

If we use 2019-01-03 in query it results in earlier problem , because the shape of "expected" and "df" remains the same also their tz is changing , but in query the tz remains constant and when we query it to diff timezone their shape becomes inconsistent as the query tz is always constant. (So it always result in +1 or -1 records and hence fails.)

Hence I think he suggested to use 2018-01-03 , as it will always result in consistent shape while checking tz-aware for all time zones.

Ah sorry I'd misunderstood, thanks!

In that case, it seems that your index for both df and expected are identical - if so, can you make a variable to share them?

Done ! Thanks.

mroeschke

LGTM. If you could merge master that will fix the failing CI

vipulr8 · 2020-05-07T03:55:36Z

LGTM. If you could merge master that will fix the failing CI

Thanks. Merging now.

updating ci/cd from master

vipulr8 · 2020-05-07T05:24:18Z

LGTM. If you could merge master that will fix the failing CI

Thanks. Merging now.

@mroeschke All checks passes.

mroeschke

LGTM

jreback · 2020-05-07T16:16:20Z

pandas/tests/frame/test_query_eval.py

+        df = pd.DataFrame(index=df_index)
+        result = df.query('"2018-01-03 00:00:00+00" < time')
+        tm.assert_frame_equal(result, expected)
+


can you also test the working case here, e.g. result = df.reset_index().query(....)

@jreback Done. Please review.

@vipinkjonwal thanks for your updates

Like this, what's the difference between df2.set_index("time") and df?

I think Jeff meant

result = df.reset_index().query('"2018-01-03 00:00:00+00" < time') expected = pd.DataFrame(df_index) tm.assert_frame_equal(result, expected)

@MarcoGorelli , I thought this was already working when the related issue was raised , I've updated the test with suggested changes.

…sue-29463

mroeschke

Sorry this PR hasn't been merged yet. Mind merging master into this branch?

After that, we can probably merge this in

…sue-29463

vipulr8 · 2020-05-20T05:04:20Z

Sorry this PR hasn't been merged yet. Mind merging master into this branch?

After that, we can probably merge this in

@mroeschke done.

mroeschke · 2020-05-20T05:40:54Z

Thanks for your patience @vipulrai91. Nice contribution!

vipulr8 · 2020-05-20T05:45:28Z

Thanks for your patience @vipulrai91. Nice contribution!

Thank you @mroeschke @MarcoGorelli @jreback for the help.
Looking forward to more contributions.

* testing issue-29463 * testing issue-29463 * adding test to close issue 29463 * making index as variable - 29463 * adding tz_aware_index_test_2 * adding tz_aware_index_test_3 Co-authored-by: Vipul Rai <“vipulrai8891@gmail.com”>

Vipul Rai added 2 commits May 6, 2020 16:38

testing issue-29463

ac8b828

testing issue-29463

e7dfe1e

MarcoGorelli reviewed May 6, 2020

View reviewed changes

mroeschke reviewed May 6, 2020

View reviewed changes

simonjayhawkins added the Testing pandas testing functions or related to the test suite label May 6, 2020

simonjayhawkins changed the title ~~Issue 29463~~ add test to close Issue 29463 May 6, 2020

adding test to close issue 29463

61efc12

MarcoGorelli suggested changes May 6, 2020

View reviewed changes

making index as variable - 29463

50b02c5

mroeschke reviewed May 7, 2020

View reviewed changes

Merge branch 'master' into issue-29464

e6cf243

updating ci/cd from master

mroeschke approved these changes May 7, 2020

View reviewed changes

mroeschke added this to the 1.1 milestone May 7, 2020

MarcoGorelli approved these changes May 7, 2020

View reviewed changes

jreback added the Timezones Timezone data dtype label May 7, 2020

jreback requested changes May 7, 2020

View reviewed changes

jreback changed the title ~~add test to close Issue 29463~~ TST: query with timezone aware index & column May 7, 2020

Vipul Rai added 4 commits May 8, 2020 01:24

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

d7aaf2d

…sue-29463

adding tz_aware_index_test_2

b7817a0

adding tz_aware_index_test_3

e4eb829

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

7fcd7a8

…sue-29463

mroeschke reviewed May 20, 2020

View reviewed changes

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

986cadd

…sue-29463

mroeschke approved these changes May 20, 2020

View reviewed changes

mroeschke merged commit 45fee32 into pandas-dev:master May 20, 2020

vipulr8 deleted the issue-29463 branch May 20, 2020 05:47

		import pandas._testing as tm


		class TestColumnvsIndexTZEquality:

Uh oh!

TST: query with timezone aware index & column #34021

TST: query with timezone aware index & column #34021

Uh oh!

Conversation

vipulr8 commented May 6, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarcoGorelli May 6, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MarcoGorelli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mroeschke left a comment

Choose a reason for hiding this comment

Uh oh!

vipulr8 commented May 7, 2020

Uh oh!

vipulr8 commented May 7, 2020

Uh oh!

mroeschke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mroeschke left a comment

Choose a reason for hiding this comment

Uh oh!

vipulr8 commented May 20, 2020

Uh oh!

mroeschke commented May 20, 2020

Uh oh!

vipulr8 commented May 20, 2020

Uh oh!

Uh oh!

MarcoGorelli May 6, 2020 •

edited

Loading