BUG: Fix IntervalIndex constructor and copy with non-default closed #18340
Conversation
Codecov Report
@@ Coverage Diff @@
## master #18340 +/- ##
==========================================
- Coverage 91.38% 91.36% -0.03%
==========================================
Files 164 164
Lines 49790 49792 +2
==========================================
- Hits 45501 45492 -9
- Misses 4289 4300 +11
Continue to review full report at Codecov.
|
happy to have this. (inline adding tests is almost always ok) |
| tm.assert_index_equal(result, expected) | ||
|
|
||
| # idempotent | ||
| tm.assert_index_equal(IntervalIndex(expected), expected) |
There was a problem hiding this comment.
can you also exercise .from_breaks and .from_arrays (can be in another test).
jschendel
force-pushed
the
iv-idx-closed
branch
from
6e1f73e to
cb89538
Compare
jschendel
force-pushed
the
iv-idx-closed
branch
from
cb89538 to
c3b6f53
Compare
| from pandas.tseries.offsets import Day | ||
| from pandas._libs.interval import IntervalTree | ||
| from pandas.tests.indexes.common import Base | ||
| import pandas.util.testing as tm | ||
| import pandas as pd | ||
|
|
||
|
|
||
| @pytest.fixture(scope='class', params=['left', 'right', 'both', 'neither']) | ||
| def closed(request): | ||
| return request.param |
There was a problem hiding this comment.
I used this fixture to avoid having @pytest.mark.parametrize('closed', ['left', 'right', 'both', 'neither']) decorators everywhere. This should be picked up by any test that has closed as an argument, without needing an explicit decorator.
|
lgtm. @shoyer can you give this a once over |
|
thanks @jschendel nice PRs as always. keep em coming! |
…andas-dev#18340) (cherry picked from commit 1915ffc)
git diff upstream/master -u -- "*.py" | flake8 --diffLumped the two issues together since they're dealing with the same thing, and the fixes are identical/small. Rewrote relevant tests to be parametrized over
closed.I also went through unrelated
IntervalIndextests and where it was straighforward parametrized overclosed. Didn't include those in the initial commit though. Not sure if it was best to include them here, or do them separately.