ENH Warn about panel.to_frame() discarding NaN GH7879

[m-novikov]

Also there was unimported module warnings with calls to it. In pandas/core/panel.py lines 718, 748 for example.

closes #7879

[cpcloud]

use not selector.all(). there's a lot of overhead with the python builtin all for ndarrays because the builtin all is very generic and works on anything with a next method. For an ndarray of size 10,000,000 ndarray.all() is about 60x faster:

In [1]: n = 1e7

In [2]: x = randn(n)

In [3]: all(x)
Out[3]: True

In [4]: x.all()
Out[4]: True

In [5]: timeit all(x)
1 loops, best of 3: 658 ms per loop

In [6]: timeit x.all()
100 loops, best of 3: 11.3 ms per loop

In [7]: 658/11.3
Out[7]: 58.23008849557522

[cpcloud]

@m-novikov couple of minor comments here, otherwise looks good

[m-novikov]

Fixed according to your comments, also added suppressing warning statement to pandas/io/tests/test_pytables.py as it's shows up on test run.

[jreback]

what does this setattr line do?

[m-novikov]

Resets warnings in case it's already been raised somewhere. So I can for sure catch this warning.

[jreback]

if u take those extra lines out ( the filtering in setup and the setattr) what happens? I am puzzled why they are in here in the first place

that is the point of assert_produces_warning it does the filtering necessary to assert the warning

[jreback]

more to the point is the rationale behind this pr
you are warning on a valid option

that seems a little odd no ?

[cpcloud]

i think this warning is ok, it's a bit of a surprise to throw away data by default.

[jreback]

why not just change the default then?

[cpcloud]

well that would be nice but would break existing code

[m-novikov]

If I lose
warnings.filterwarnings(action='ignore', category=UserWarning)
then during test run there will be this warning raised.
If I lose
setattr then I will be dependent on order in which tests are run, warning raised only once by default, and if it was raised in some other test module I will not catch it.

[jreback]

I say just a change the default as an API change

or do nothing

[cpcloud]

i'd be ok with that, maybe a nice warning in the docs that says "you have to call dropna now"

[m-novikov]

Removed warnings suppression

[jreback]

@m-novikov if u change the result does anything break in the test suite (aside from the actual test for this)

[m-novikov]

@jreback if I leave it with warnings nothing breaks, just occasional message in some test will be displayed and so without it but as was stated it's not expected behaviour.
setattr(panelm, '__warningregistry__', {}) breaks nothing, but warnings of this module may be displayed twice during test suite run. Possibly I should store state of registry and restore it after test.

[jreback]

no I mean if u change the default to False
instead if warning

[m-novikov]

@jreback There is SparsePanel.to_frame() which doesn't support filter_observations=False, if change signature only for Panel class then it will be inconsistent.
Also it breaks a few methods like Panel.count for example.

[jreback]

@m-novikov can you put in a short doc note. Also Let's change this to a FutureWarning. Then change the default in 0.17.0

[m-novikov]

@jreback updated

[jreback]

I would like to repurpose this and simply change the default here (no need to warn then). Can you update?

[m-novikov]

Yes, so this means all related test to be fixed too, I will get to it

[jreback]

@m-novikov gr8! lmk

[jreback]

I think we can simply change this for 0.17.0.

going to close this, but pls reopen if you want address that.