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

Clarify descriptions of --normalize-by-specimen flag functionality in scripts/classif_rect.py #312

Merged
merged 5 commits into from
Feb 3, 2014

Conversation

cmccoy
Copy link
Collaborator

@cmccoy cmccoy commented Nov 21, 2013

The functionality in question is much easier to understand if "normalized tally" for a specimen is described as the "frequency" of some taxon for that specimen. As such, the "normalized_tally" output in by_taxon can be described as an "average_frequency".

@ghost ghost assigned metasoarous Oct 24, 2013
@metasoarous
Copy link
Collaborator Author

I've realized this functionality was only in the the three-study branch, so will be merging this in as part of this ticket as well.

@metasoarous
Copy link
Collaborator Author

This task ballooned a bit into a bit of an overhaul for increased consistency and ease of understanding what the output/input options are. See branch 312-classif-rect-average-frequency for the current status.

@nhoffman - There are a few other things we chatted about that would be nice, but these might have to wait. I can open up separate ticket(s) for those though.

…d, tax_name or rank (this could break things, and would be silly anyway).
@nhoffman
Copy link
Collaborator

@metasoarous - thanks for this - is this ready for a pull request? I'm updating the pipeline (depends on changes already in dev) and it would be nice to start using this. If not, no problem - I can always grab the script and use it standalone.

@metasoarous
Copy link
Collaborator Author

@matsen - Noah and I just chatted about this. While these changes are ready to be merged, they are breaking changes. Since we've also renamed the script (as classif_table.py), we're wondering if it doesn't make sense to leave the old script in and deprecate it with a warning that the script will be removed eventually. What do you think?

@matsen
Copy link
Owner

matsen commented Nov 21, 2013

Sounds great!

On Thu, Nov 21, 2013 at 1:40 PM, Christopher Small <notifications@github.com

wrote:

@matsen https://github.com/matsen - Noah and I just chatted about this.
While these changes are ready to be merged, they are breaking changes.
Since we've also renamed the script (as classif_table.py), we're
wondering if it doesn't make sense to leave the old script in and deprecate
it with a warning that the script will be removed eventually. What do you
think?


Reply to this email directly or view it on GitHubhttps://github.com//issues/312#issuecomment-29027232
.

Frederick "Erick" Matsen, Assistant Member
Fred Hutchinson Cancer Research Center
http://matsen.fhcrc.org/

@metasoarous
Copy link
Collaborator Author

Good to merge?

@matsen
Copy link
Owner

matsen commented Feb 3, 2014

@nhoffman shall I just click on the big green button?

@nhoffman
Copy link
Collaborator

nhoffman commented Feb 3, 2014

Yes!

On Mon, Feb 3, 2014 at 10:14 AM, Erick Matsen notifications@github.comwrote:

@nhoffman https://github.com/nhoffman shall I just click on the big
green button?

Reply to this email directly or view it on GitHubhttps://github.com//pull/312#issuecomment-33982732
.

matsen added a commit that referenced this pull request Feb 3, 2014
Clarify descriptions of `--normalize-by-specimen` flag functionality in scripts/classif_rect.py
@matsen matsen merged commit 4265c33 into dev Feb 3, 2014
@matsen matsen deleted the 312-classif-rect-average-frequency branch February 3, 2014 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants