-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
index into multi-index past the lex-sort depth #8526
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
Conversation
we'll put this on tap for 0.15.1. Even though it is a slight API change. will need to add an example at some point. further, pls put a replica of the tests in the issue (as well as your complete test) |
caebcc7
to
e31cb4f
Compare
updated the pr with some examples |
e31cb4f
to
2e48fea
Compare
if keylen == self.nlevels and self.is_unique: | ||
def _maybe_str_to_time_stamp(key, lev): | ||
if lev.is_all_dates and not isinstance(key, Timestamp): | ||
try: return Timestamp(key, tz=getattr(lev, 'tz', None)) |
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 put on sep lines (I know its trivial, but more readable)
ok minor comments (and I seem to remember need an API example in v0.15.1 (to show off the indexing beyon lex-sort depth). looks gr8 otherwise |
e36a794
to
6e79d54
Compare
could not find a similar functionality to |
pls update the docs to something like this (the v0.15.1). I had done this but then realized that the warning is not tested, so
|
eb83bd3
to
06611ed
Compare
hmm it is kind of finicky but should work another way of testing this is and see if it is failing for some reason |
@jreback what shall we do regarding the warnings test? |
mask &= df.iloc[:, i] == k | ||
|
||
if not mask.any(): | ||
assert key[:i+1] not in mi.index, \ |
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.
use self.assertIn
(or self.assertTrue
); these are more informative that a bare assert
not sure that is the problem. It seems some indexing past lexsort_depth of 0 works (or at least works but doesn't show the warning). And the example that you are trying to catch the warning, does seem to work for me. |
cd47eef
to
05ea639
Compare
it may be the case that you are specifying a full key, since if it is not a partial key and the index is unique it takes a different code path. if you have an example which this is not the case, let me know and i will look into it. |
df.index.lexsort_depth | ||
|
||
# in prior versions this would raise a KeyError | ||
# will now show a RuntimeWarning |
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.
this is now PerformanceWarning, yes?
@behzadnouri ok, minor doc change. lmk when you are ready (e.g. you are investigating that last issue) |
17fbb2a
to
1fa414e
Compare
fixed the doc ( |
This warning has to be tested. See the example below.
But doing it with an even greater depth is ok?
|
@jreback my guess is that you did not set the warnings filter to this is on python 3.4:
|
well, this has to work on py2.7 as well (and test that way). (and I directly tried this in a single ipython session). something still not right. |
i am almost sure this is python bug, which is fixed in newer versions. i already linked to issues on python bug tracker, and have given up on making this work on older versions of python. I mean i understand the necessity for checking for warnings but i could not make it work; esp. my home computer is python 3.4, and i cannot reproduce it at home. if you add |
Then make another test which does exactly what your whatsnew example does (which shows the warning properly). You can explicity turn the warnings filter on and off. The python bug is completely irrelevant; the warning DOES happen. You just need to assert that is happening when you think it should happen. Otherwise when someone else makes a change to this code and the warning disappears it will not be caught (whether on purpose or not). The key is that travis tests the warning. 2.7 is well in use and will be for the foresable future. You can easily install 2.7 using |
d75cb01
to
e74c4a0
Compare
i added a new test just to test the warning, as in the example above; |
df2 = df.sortlevel() | ||
df2 | ||
df2.index.lexsort_depth | ||
df.loc[(1,'z')] |
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.
this should be df2?
e74c4a0
to
d0861e8
Compare
yes, fixed that |
@behzadnouri thanks for this! I had to add a method of clearing the warnings (added to assert_produce_warning) as was failing thanks! |
closes #7724
closes #2646
on current master:
False negative if the key length exceeds lexsort depth:
only ones which work:
which take a different code paths. The last one only works if the index is unique:
for all of the false negative cases above, obviously
df.loc[key]
fails:Some of these issues persist even if the index is lexically sorted:
date-time indexing with a full-key fails if index is unique:
also, non-unique lexically sorted index always returns false positive with any full key: