-
Notifications
You must be signed in to change notification settings - Fork 40
Creates warnings when merging tables with different taxon depth or one level of depth. #343
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
base: dev
Are you sure you want to change the base?
Conversation
|
Hey @Macabe222 is this ready for a review? |
|
@lizgehret Yes it is |
|
Sounds good, thanks @Macabe222! We'll get someone to look at this either this week or next 🙂 |
|
|
||
| def merge_taxa(data: pd.DataFrame) -> pd.DataFrame: | ||
| data = _merge_feature_data(data) | ||
| if isinstance(data, list): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
data is always a list because it's typed as one (see plugin_setup.py)
| frame_one = data[0] | ||
| frame_two = data[1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't work for multiple reasons. First, the input is a list even if there is only one taxonomy passed in--this will give an index error for data[1]. Second, this doesn't handle the case where more than two taxonomies are being merged.
| count = frame_one["Taxon"].str.count(';') | ||
| count_two = frame_two["Taxon"].str.count(';') | ||
|
|
||
| for index, value in count.items(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What this does is see if the two taxonomies have the same number of levels for each pair of taxa, matched by index. This is meaningless because taxon index has no meaning. What the issue is requesting is to see if each of the merged taxonomies as a whole has the same number of levels (maximum depth) as the others.
| except KeyError: | ||
| continue | ||
|
|
||
| if (count == 0).any(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not what the issue is requesting. Go read the forum post more carefully.
Adds warnings for merging tables with differing levels of taxonomic depth, and adds a warning when merging tables with one one level of taxonomic depth in
merge_taxa.Addresses #341