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

Filter elements with zero sample size in fathpd #53

Merged
merged 5 commits into from
Oct 9, 2023

Conversation

sfiligoi
Copy link
Collaborator

@sfiligoi sfiligoi commented Oct 6, 2023

Elements with zero sample size have no clear semantics in faithpd, so we filter them out.
But there was a historical assumption that a single zero element is allowed and would evaluate to 0, so we preserve that option, but issue a warning.

@sfiligoi sfiligoi changed the title Filter elements with zero sample size in fathead Filter elements with zero sample size in fathpd Oct 6, 2023
@sfiligoi sfiligoi mentioned this pull request Oct 6, 2023
@sfiligoi
Copy link
Collaborator Author

sfiligoi commented Oct 6, 2023

This addresses #51

@sfiligoi
Copy link
Collaborator Author

sfiligoi commented Oct 6, 2023

CC @vpa1977

@sfiligoi
Copy link
Collaborator Author

sfiligoi commented Oct 6, 2023

I checked that the unifrac python tests still work.

@vpa1977 Can you check if this fixes the problems in #51 for you, since I cannot reproduce it myself?

@vpa1977
Copy link

vpa1977 commented Oct 8, 2023

Thank you!!!!

I am getting

$python3 unifrac/tests/test_api.py 

WARNING: All samples had zero counts. Forcing zero result.
.
----------------------------------------------------------------------
Ran 1 test in 0.014s

OK

and in the debugger everything looks right.

I wonder if it would make sense to add some test for "WARNING: Some samples had zero counts and were filtered out.\n", or it is an edge condition that does not occur in the reality?

@wasade
Copy link
Member

wasade commented Oct 9, 2023

Unlikely in real use, thanks!

@sfiligoi sfiligoi merged commit 1834378 into biocore:main Oct 9, 2023
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.

None yet

3 participants