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

merge binomen into this package #12

Closed
sckott opened this issue Dec 5, 2016 · 15 comments · Fixed by #66
Closed

merge binomen into this package #12

sckott opened this issue Dec 5, 2016 · 15 comments · Fixed by #66
Assignees
Milestone

Comments

@sckott
Copy link
Contributor

sckott commented Dec 5, 2016

from #6

@sckott sckott added this to the v0.1 milestone Dec 5, 2016
@sckott sckott self-assigned this Dec 13, 2016
@sckott
Copy link
Contributor Author

sckott commented Dec 17, 2016

i'm waiting on this until major changes are merged in in https://github.com/ropenscilabs/taxa/tree/issue-9

@zachary-foster
Copy link
Collaborator

Ok. The only thing that perhaps should get done is adding examples to the documentation for taxonomy, subtaxa, supertaxa, roots, and stems. Are you going to do that? I can also do it. Either way is fine with me. We can also merge now if you want; everything is working as far as I can tell.

@sckott
Copy link
Contributor Author

sckott commented Dec 19, 2016

Right on the examples, but most of your changes are not on master branch right? So I want to merge those in before merging together with binomen.

for examples happy to add some, but again, would be nice to merge in your changes first.

can you send PR soon?

@zachary-foster
Copy link
Collaborator

Oh yea, its ready to merge. I assumed you would make the changes on the issue-9 branch, but a PR makes more sense. I will send it now

@sckott
Copy link
Contributor Author

sckott commented Dec 19, 2016

thanks, i wasn't sure which branch was the most recent

@sckott
Copy link
Contributor Author

sckott commented Apr 25, 2017

i'm going to ask CRAN to archive binomen this week since we're rolling into this pkg, and it will fail on CRAN checks with a new dplyr coming out on 11 May

-- scratch that - forget that we were going to rename pkg to binomen - not sure if we'll have this pkg ready by 11 may, so i'll make a fix for new dplyr for binomen on cran

@zachary-foster
Copy link
Collaborator

Thanks for the update

@sckott
Copy link
Contributor Author

sckott commented May 11, 2017

working on branch include-binomen

sckott added a commit that referenced this issue May 11, 2017
so far just pop, and only supporting Hierarchy objects right now
@sckott
Copy link
Contributor Author

sckott commented May 11, 2017

@zachary-foster What do you think? See ?pop and ?hierarchy - thoughts on how that's implemented and how it behaves? good to decide on how we should structure these, then can roll out the rest of the functions following the same structure

@sckott
Copy link
Contributor Author

sckott commented May 11, 2017

oh, and install from include-binomen

@zachary-foster
Copy link
Collaborator

zachary-foster commented May 11, 2017

Cool, I will look at this later today

@sckott
Copy link
Contributor Author

sckott commented May 11, 2017

thx

@zachary-foster
Copy link
Collaborator

Looks good. Perhaps rename to something that mentioned rank? Like pop_rank? Or make usable with names and ids too?

It would be cool if we could do something like pop(data, species:family) too

When I was looking at this, I thought we might be able to add an abstracted version as well that can filter by taxon name and taxon ids or any other information we can derive from the class. We might be able to reuse a lot of code from taxmap to make an abstracted filtering function like filter_taxa for hierarchy. The basic idea would be:

  • A set of simple accessor functions for per-taxon attributes that each return a vector. e.g. names,ids, ranks, authorities`.
  • Equivalent to the taxmap functions get_data and get_names that together can find variable names from expressions and return the relevant data from the accessor functions.
  • A function like filter_taxa that can take any number of expressions that resolve to TRUE/FALSE or index vectors and subset the taxa accordingly.

For example:

filter_taxa(hier_obj, ranks == "species", ids %in% c(123, 456))

We could also add subtaxa and supertaxa options.

@zachary-foster
Copy link
Collaborator

Thinking about this more, the filter_taxa syntax could be apllied to all of the classes with multiple taxa like 'taxa' 'hierarchy' 'hierarchies' and 'taxonomy'. All of the NSE code can be reused and having a 'get_data' method for each class would make converting class info to tables a simple wrapper over get_data

@sckott
Copy link
Contributor Author

sckott commented May 13, 2017

Or make usable with names and ids too?

that sounds good, though we'll probably only be able to do a subset of operations with names and ids since there's no ordering to them

It would be cool if we could do something like pop(data, species:family) too

this is the operation strain in binomen, haven't ported that over yet

Will have a look at filter_taxa

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 a pull request may close this issue.

2 participants