BUG: Fix IntervalTree handling of NaN

[jschendel]

• closes BUG: IntervalTree should not have NaN in nodes #23352
• tests added / passed
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

xref #23327 : As far as I can tell the only reason that PR is failing is because of tests raising a RuntimeWarning due to this bug.

[pep8speaks]

Hello @jschendel! Thanks for submitting the PR.

• There are no PEP8 issues in the file pandas/tests/indexes/interval/test_interval_tree.py !

[mroeschke]

Does this apply to NaT as well?

[jschendel]

Does this apply to NaT as well?

IntervalIndex performs an i8 conversion on datetime/timedelta data prior to passing it to IntervalTree, as IntervalTree only supports numeric dtypes, so only need to worry about np.nan in IntervalTree. As part of the i8 conversion NaT is explicitly converted to np.nan instead of NaT's integer value (implemented in #23327 but could maybe be moved here), as it's integer value can be mistakenly interpreted as non-null data once everything's been converted to integers.

[codecov]

Codecov Report

Merging #23353 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #23353      +/-   ##
==========================================
- Coverage   92.22%   92.18%   -0.05%     
==========================================
  Files         169      161       -8     
  Lines       51258    51160      -98     
==========================================
- Hits        47274    47161     -113     
- Misses       3984     3999      +15

Flag	Coverage Δ
#multiple	90.61% <ø> (-0.05%)	⬇️
#single	42.24% <ø> (+0.01%)	⬆️

Impacted Files	Coverage Δ
pandas/util/_depr_module.py	0% <0%> (-65.12%)	⬇️
pandas/core/indexes/period.py	91.04% <0%> (-1.24%)	⬇️
pandas/core/dtypes/common.py	94.37% <0%> (-0.49%)	⬇️
pandas/core/dtypes/cast.py	88.92% <0%> (-0.31%)	⬇️
pandas/core/arrays/timedeltas.py	94.3% <0%> (-0.18%)	⬇️
pandas/core/nanops.py	95.05% <0%> (-0.15%)	⬇️
pandas/core/arrays/period.py	97.48% <0%> (-0.09%)	⬇️
pandas/core/series.py	93.89% <0%> (-0.03%)	⬇️
pandas/io/pytables.py	92.43% <0%> (-0.01%)	⬇️
pandas/core/generic.py	96.79% <0%> (-0.01%)	⬇️
... and 21 more

---

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 437f31c...552accb. Read the comment docs.

[jreback]

what exactly does this produce though? are the nan's valid? you are just dropping them.

[jschendel]

The intent here was to verify that the RuntimeWarning described in the issue is not occurring. I guess this test isn't really necessary, as at least one CI instance is set to fail if warnings are raised. Removed this test, and instead parametrized the tree fixture to include leaf_size that will trigger the warning and cause the invalid behavior described in the issue (so beyond just a warning would cause the asserts to fail if there's a regression).

[jreback]

this broke 32 bit builds
https://travis-ci.org/MacPython/pandas-wheels/jobs/448674095

can u have a look @jschendel

[jschendel]

@jreback : it looks like this 32 bit failure is something that's come up before and is currently explicitly skipped in the other test where it occurs:

pandas/pandas/tests/indexes/interval/test_interval_tree.py

Lines 88 to 91 in de39bfc

	@pytest.mark.skipif(compat.is_platform_32bit(),
	reason="int type mismatch on 32bit")
	@pytest.mark.parametrize('leaf_size', [1, 10, 100, 10000])
	def test_get_indexer_closed(self, closed, leaf_size):

This was done before my time, so not sure about the exact history here.

My plan is to similarly skip the newly written tests and open a relevant issue to formally document this. It doesn't seem particularly pressing to fix since this has been present for a while, only impacts 32 bit, and there haven't been any issues opened. Can look into fixing it if I'm mistaken, though I don't immediately see what needs to be done.