Skip to content
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

Adding a warning when dropping NA values for panel.to_frame #7879 #8063

Closed
wants to merge 6 commits into from

Conversation

Magellanea
Copy link

closes: #7879

I've added a warning when the NaN values are dropped when calling pd.Panel(dict).to_frame() with the param filter_observations true

I've supplied a test case to be used against the example provided in the issue tracker

@@ -858,6 +858,9 @@ def to_frame(self, filter_observations=True):
mask = com.notnull(self.values).all(axis=0)
# size = mask.sum()
selector = mask.ravel()
if not np.all(selector):
warnings.warn("NaN values found\
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll need to import warnings in this file too

@TomAugspurger
Copy link
Contributor

Thanks for the PR.

I added a couple comments. Let me know if they don't make sense or you need any help.

@jreback
Copy link
Contributor

jreback commented Aug 18, 2014

their is another pr for this issue

however I think in that one the consensus was to make an API change to make filter_observations to False

instead of doing a warning which I think is a bit odd
as we don't warn on anything else like this

@Magellanea
Copy link
Author

@TomAugspurger, I've modified the test as following

def test_to_frame_na_drop_warnings(self):
        df1 = DataFrame(np.random.randn(2, 3), columns=['A', 'B', 'C'],
               index=['foo', 'bar'])
        df2 = DataFrame(np.random.randn(2, 3), columns=['A', 'B', 'C'],
               index=['foo', 'bar'])
        df2.loc['foo', 'B'] = np.nan
        dict_without_dropped_vals = {'df1': df1, 'df2': df2}
        ## A panel without dropped vals shouldn't throw warnings
        with tm.assert_produces_warning(False):
            Panel(dict_without_dropped_vals).to_frame()
        ## A panel with dropped vals should throw a Runtime warning if \
        # filter_observations is True
        df2_with_na_vals = DataFrame(df2)
        df2_with_na_vals.loc['foo', 'B'] = np.nan
        dict_with_dropped_vals = {'df1': df1, 'df2_dropped': df2_with_na_vals}
        with tm.assert_produces_warning(RuntimeWarning):
            Panel(dict_with_dropped_vals).to_frame()
        ##if filter_observations is False, a warning shouldn't be throws
        with tm.assert_produces_warning(False):
            Panel(dict_with_dropped_vals).to_frame(filter_observations=False)

Does this match what you expected?

Thanks for the reply

@Magellanea
Copy link
Author

@jreback So you suggest I stop working on that PR as pandas doesn't use warnings by default, Right?
Concerning the other PR, was it accepted, I didn't find any PRs or comments referencing it?
Thanks

@jreback
Copy link
Contributor

jreback commented Aug 18, 2014

see #7970. Its not been accepted as of yet.

pandas does use warnings. Its for this usecase.

I think the API change is better. Really just looking for some feedback (and then impl of course).

@jreback
Copy link
Contributor

jreback commented Aug 18, 2014

@Magellanea pls review that PR and post comments (here is ok).

@Magellanea
Copy link
Author

@jreback Concerning the API change - if I'm we're going to neglect the warnings - do you suggest that we evaluate filter_observation to be false regardless of the passed value, Is that what you mean by the API CHANGE, Thanks

@jreback
Copy link
Contributor

jreback commented Aug 22, 2014

@Magellanea no I meant change the default of filter_observations=True to False. I think this would be fine as an API change with an example in v0.15.0.txt. You will have a couple of test fialures I think which will need addressing.

@jreback jreback modified the milestones: 0.15.1, 0.15.0 Sep 9, 2014
@jreback
Copy link
Contributor

jreback commented Sep 9, 2014

@Magellanea can you rebase / address comments?

@Magellanea
Copy link
Author

@jreback I'm sorry for leaving this for a while, the problem was that the travis built failed, and I don't know what the reason was, I've changed the filter_observation default value and in the corresponding test but the built failed for other reasons and irrelevant tests

@jreback
Copy link
Contributor

jreback commented Jan 25, 2015

dupe of #7970

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Design Reshaping Concat, Merge/Join, Stack/Unstack, Explode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

panel.to_frame() discards nan by default
3 participants