Default values for dropna to "False" (issue 9382)

[nickeubank]

PLEASE REVIEW: This is my commit to a major project, and would appreciate a quick once over!

As per discussion in Issue 9382, changes all HDF functions from having default of dropping all rows with NA in all non-index rows.

closes #9382

[jreback]

all updates should be in this PR.

[jreback]

odd that no tests fail. pls come up with a tests that fails w/o your change and succeeds with it. I guess this was not tested.

[nickeubank]

OK, both on this branch.

Will look into testing -- I'm not really sure what that means, sorry.

[nickeubank]

Sorry Jeff, I think testing may be beyond my pay-grade. I thought I knew enough to contribute, but the testing wiki ( https://github.com/pydata/pandas/wiki/Testing ) is all greek to me. This is obviously not a big commit, so please feel free to just close the pull request and issue and ignore all this. Sorry. :/

[bashtage]

@nickeubank

Just write a function that matches the patters

def test_all_missing_values

That saves an reloads an array that would drop under the previous and won't drop now.

Test should be here:

https://github.com/pydata/pandas/blob/master/pandas/io/tests/test_pytables.py#L4680

[nickeubank]

Thanks @bashtage -- and where would I place this function?

And sounds like it needs to raise an AssertionError with descriptions for source of problem if it the calls lead to the rows being dropped when they shouldn't be?

[bashtage]

The test should have the followign properties:

1. The df should satisfy the condition that some rows woudl be dropped if saved with master
2. You should save the df with the code in your PR, using the default options for saving.
3. Verify that all rows are present using assert_frame_equal(loaded_df, original_df)

Item 3 would fail on master since the loaded df will be missing the all nan row in the original df. This will, however, not fail with your PR.

See my link about where your function goes. If you want to check that it passes, you can run

nosetests pandas.io.tests.test_pytables

Assuming you are using python setup.py develop to install your local pandas.

[nickeubank]

Ah, ok great. And do I just stick this into pytables.py, or is there a special place for these?

(Again, so sorry to need the hand holding!)

[bashtage]

Ah, ok great. And do I just stick this into pytables.py, or is there a special place for these?

pandas/io/tests/test_pytables.py

There is almost always a test folder and a corresponding file names test_xxx for every .py file.

[nickeubank]

Excellent, thanks. Will do now.

[jreback]

• show these examples with a DataFrame with a row that has all-nans, and one that doesnt.
• don't use camel case.
• use to_hdf/read_hdf (because for example you are not closing the store)
• this needs to be a code-block

[nickeubank]

OK -- I've made all the suggested updates, and I have a build failure. I'm not sure how to check if that's due to the test I added, but hopefully that's what we were looking for?

[bashtage]

Your code. pd is not imported. Just use read_hdf directly.

ERROR: test_all_missing_values (pandas.io.tests.test_pytables.TestHDFStore)
----------------------------------------------------------------------

Traceback (most recent call last):

File "/home/travis/build/pydata/pandas/pandas/io/tests/test_pytables.py", line 4682, in test_all_missing_values

df_with_missing = pd.DataFrame({'col1':[np.nan]})

NameError: global name 'pd' is not defined

[bashtage]

You should probably also include a test with a slightly more complicated layout, e.g. with a full row, a row with some mulls and a row with all null.

[nickeubank]

@bashtage - thanks. Just found the traceback in Travis -- will check next build myself!

[jreback]

you don't need print statements, paste the actual ipython output (which will have numbered In/Outs)

[jreback]

closing pls reopen if/when updated

[nickeubank]

Sorry, forgot about this. Now updated and rebased. How do I reopen? Or do I need to start a new PR?

[bashtage]

Seems straightforward and ready.

[nickeubank]

@bashtage moved to #10097 -- updating for a @jreback comment now.