Skip to content

Conversation

@andrewsanchez
Copy link
Contributor

No description provided.

This makes it easier to see what's going on below with the values
returned by this function.
This prevents attempting to drop columns that don't exist in merged.columns
after setting the index, while still dropping columns that are present in
merged.columns.  Attempting to do so raises an exception in pandas >= 1.

Please see pandas-dev/pandas#8594
for details.
# Removes the column name used to set the index of `merged` above
col_diff = set(columns) - set([column])
if col_diff:
counts.drop(col_diff, axis=1, inplace=True, level=0)
Copy link
Contributor Author

@andrewsanchez andrewsanchez Feb 13, 2020

Choose a reason for hiding this comment

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

The first kwarg taken by DataFrame.drop is labels and it accepts a single label or list-like. Passing the set directly seems to work fine here, but we can make it a pd.Index or list if that is preferable.

Copy link
Member

Choose a reason for hiding this comment

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

nah, this looks good to me

This avoids attempting to drop columns that had already been dropped in previous
calls to _reindex_with_metadata in the for loop in `alpha_rarefaction`.

Co-authored-by: Matthew Dillon <[email protected]>
counts = merged.count()
counts.drop(columns, axis=1, inplace=True, level=0)
median_ = merged.median()
reindexed = merged.set_index(column)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Avoid mutating merged by assigning to a new DataFrame reindexed.

Copy link
Member

@nbokulich nbokulich left a comment

Choose a reason for hiding this comment

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

Thanks @andrewsanchez ! please see my inline comment.

@nbokulich
Copy link
Member

Thanks for clarifying... LGTM

@nbokulich nbokulich merged commit b894b00 into qiime2:master Feb 14, 2020
@andrewsanchez andrewsanchez deleted the pandas-1-fixes branch February 14, 2020 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants