-
-
Notifications
You must be signed in to change notification settings - Fork 18.3k
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: #15752 Add test_drop_duplicates for Categorical dtypes #18072
TST: #15752 Add test_drop_duplicates for Categorical dtypes #18072
Conversation
Hello @tmnhat2001! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on November 06, 2017 at 02:47 Hours UTC |
Codecov Report
@@ Coverage Diff @@
## master #18072 +/- ##
==========================================
- Coverage 91.25% 91.23% -0.02%
==========================================
Files 163 163
Lines 50120 50120
==========================================
- Hits 45737 45727 -10
- Misses 4383 4393 +10
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #18072 +/- ##
==========================================
- Coverage 91.25% 91.23% -0.02%
==========================================
Files 163 163
Lines 50120 50120
==========================================
- Hits 45737 45727 -10
- Misses 4383 4393 +10
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #18072 +/- ##
==========================================
+ Coverage 91.25% 91.26% +<.01%
==========================================
Files 163 163
Lines 50120 50122 +2
==========================================
+ Hits 45737 45743 +6
+ Misses 4383 4379 -4
Continue to review full report at Codecov.
|
pandas/tests/test_categorical.py
Outdated
@pytest.mark.parametrize( | ||
"input1, input2, cat_array", | ||
[ | ||
( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so instead of duplicating code like this you can use multiple parametrize
just parametrize on the dtype and construct inside
something like
@pytest.mark.parmetrize(
"consturctor", ['int_', ....])
def test_drop_duplicates_non_bool(self, input):
input1 = np.array([1, 2, 3, 3], dtype=dtype)
....
this will work for datetimes and timedeltas as well (use 'M8' and 'm8')
Thanks, that makes the test a lot cleaner. While making changes to the test, I noticed an issue with creating a series of datetime64[D] categorical.
pd.show_versions():INSTALLED VERSIONS ------------------ commit: e1dc3c9 python: 3.6.3.final.0 python-bits: 64 OS: Windows OS-release: 10 machine: AMD64 processor: Intel64 Family 6 Model 61 Stepping 4, GenuineIntel byteorder: little LC_ALL: None LANG: None LOCALE: None.None pandas: 0.22.0.dev0+41.ge1dc3c9 |
@tmnhat2001 hmm that looks like a bug. can you mark that xfail, then we can merge this and can address in a folllowup. |
@tmnhat2001 I think this is the issue: #7996 |
Yeah, that looks like it. There's no issue if datetime64[h] is used. Can we use that instead? |
@@ -797,6 +797,96 @@ def test_set_categories_inplace(self): | |||
cat.set_categories(['a', 'b', 'c', 'd'], inplace=True) | |||
tm.assert_index_equal(cat.categories, pd.Index(['a', 'b', 'c', 'd'])) | |||
|
|||
@pytest.mark.parametrize( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add an xfail with datetime64[D], something like this
"timedelta64[h]", pytest.param('datetime64[D]', marks=pytest.mark.xfail(
reason='write the issue number'))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, just added it
thanks @tmnhat2001 |
certainly take PR's for the fix for xfail here or any other issues you'd like to work on! |
thanks for helping! |
git diff upstream/master -u -- "*.py" | flake8 --diff