Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TST: Clean old timezone issues PT2 #21612

Merged
merged 10 commits into from
Jun 28, 2018
Merged

Conversation

mroeschke
Copy link
Member

@mroeschke mroeschke added Testing pandas testing functions or related to the test suite Timezones Timezone data dtype labels Jun 23, 2018
@mroeschke mroeschke added this to the 0.24.0 milestone Jun 23, 2018
@mroeschke mroeschke changed the title TST: Old timezone issues PT2 TST: Clean old timezone issues PT2 Jun 23, 2018
@codecov
Copy link

codecov bot commented Jun 23, 2018

Codecov Report

Merging #21612 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #21612      +/-   ##
==========================================
+ Coverage    91.9%    91.9%   +<.01%     
==========================================
  Files         154      154              
  Lines       49555    49558       +3     
==========================================
+ Hits        45542    45545       +3     
  Misses       4013     4013
Flag Coverage Δ
#multiple 90.27% <ø> (ø) ⬆️
#single 42.02% <ø> (-0.01%) ⬇️
Impacted Files Coverage Δ
pandas/core/frame.py 97.19% <0%> (ø) ⬆️
pandas/core/indexes/interval.py 93.16% <0%> (+0.03%) ⬆️

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 73ab162...0f63b69. Read the comment docs.

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mroeschke can you break to 2 PRs, 1 which moves the issues out of 0.23.2 (just because want to cut 0.23.2 soon, so need to move)

operator.ge, operator.gt]


@pytest.fixture(params=COMPARISON_OPERATORS)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make this more like this

+@pytest.fixture(params=['__eq__', '__ne__', '__le__',
+                        '__lt__', '__ge__', '__gt__'])
+def all_compare_operators(request):
+    """
+    Fixture for dunder names for common compare operations
     """

which is much more flexible for non-operators

@@ -2248,6 +2262,16 @@ def test_setitem_datetimelike_with_inference(self):
index=list('ABCDEFGH'))
assert_series_equal(result, expected)

@pytest.mark.parametrize('idxer', ['var', ['var']])
@pytest.mark.parametrize('tz', [None, 'UTC'])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

try to use the tz fixtures if possible

@@ -1185,3 +1185,11 @@ def test_constructor_range_dtype(self, dtype):
expected = Series([0, 1, 2, 3, 4], dtype=dtype or 'int64')
result = Series(range(5), dtype=dtype)
tm.assert_series_equal(result, expected)

def test_constructor_tz_aware_and_tz_naive_data(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call this tz_mixed

@@ -249,3 +249,12 @@ def test_replace_mixed_types_with_string(self):
result = s.replace([2, '4'], np.nan)
expected = pd.Series([1, np.nan, 3, np.nan, 4, 5])
tm.assert_series_equal(expected, result)

@pytest.mark.parametrize('to_replace', [pd.NaT, [np.nan, pd.NaT]])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have a fixture for nulls

@@ -1688,6 +1688,20 @@ def test_getitem_list_duplicates(self):
expected = df.iloc[:, 2:]
assert_frame_equal(result, expected)

def test_getitem_setitem_datetimeindex_tz(self):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there are other tests like this, can you co-locate (IIRC in same file), maybe though in pandas/tests/indexing/....

@@ -292,6 +292,15 @@ def test_construct_over_dst(self):
freq='H', tz='US/Pacific')
tm.assert_index_equal(result, expected)

def test_construct_with_different_start_end_string_format(self):
# GH 12064
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure this is actually testing what is in the issue, as there were provided tz's there.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tz aren't provided in these examples via the keyword arg. The issue was that '2013-01-01 00:00:00+09:00' vs '2013/01/01 02:00:00+09:00' (note the - vs the /) ended up produced different tzoffsets

#12064 (comment)

@@ -450,6 +450,14 @@ def test_offset_deprecated(self):
with tm.assert_produces_warning(FutureWarning):
idx.offset = BDay()

def test_ts_datetimeindex_compare_mismatched_tz(self, comparison_fixture):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have similar tests, can you co-locate

@@ -249,3 +249,12 @@ def test_replace_mixed_types_with_string(self):
result = s.replace([2, '4'], np.nan)
expected = pd.Series([1, np.nan, 3, np.nan, 4, 5])
tm.assert_series_equal(expected, result)

@pytest.mark.parametrize('to_replace', [pd.NaT, [np.nan, pd.NaT]])
def test_replace_with_tz_aware_data(self, to_replace):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you tests with tz-naive as well, maybe just have a combined test (as we likely have one for naive)

@mroeschke
Copy link
Member Author

Addressed your comments @jreback and all green.

@jreback jreback merged commit e0f978d into pandas-dev:master Jun 28, 2018
@jreback
Copy link
Contributor

jreback commented Jun 28, 2018

thanks @mroeschke had some duplicate whatsnew entries, but fixed

@mroeschke mroeschke deleted the test_tz_issues branch June 28, 2018 15:06
@mroeschke mroeschke mentioned this pull request Jun 29, 2018
6 tasks
Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment