search

ropensci / taxa

taxonomic classes for R
https://docs.ropensci.org/taxa
Other
48 stars 12 forks source link

added `classified` class and examples #1

Closed zachary-foster closed 8 years ago

zachary-foster commented 8 years ago

Here is my idea for a class for data classified by a taxonomy. This would be used to store taxonomic information for a set of sequences for example. Its still a bit rough and incomplete, but it should be enough to get the idea. Let me know if you have any questions.

zachary-foster commented 8 years ago

Oh yea, look at the vignette for examples.

sckott commented 8 years ago

great, I'll have a look

zachary-foster commented 8 years ago

Thanks! Note that I just pushed another commit that fixed a few R CMD Check errors, but one is still there. I cant figure out how to get rid of it without breaking code at the moment. Also, I had put the example vignette in the root directory instead of the vignette directory on accident. I moved it to the vignette directory.

zachary-foster commented 8 years ago

FYI @sckott, I am adapting metacoder and taxa to work together before hearing back from you since I am in a rush to get metacoder published. Some of the functions in this PR were copied directly from metacoder. If you don’t think that this system is a good fit for a taxa/binomen its no big deal, I will just put the contents of this PR in metacoder and wait to see if we come up with something that will fit the needs of metacoder in the future.

sckott commented 8 years ago

sorry for delay on this. will have a look

I am in a rush to get metacoder published

do you need taxa on CRAN soon for metacoder to work?

zachary-foster commented 8 years ago

No problem, just didn’t want to you to feel pressured to use this system because I am moving ahead with it. The code will get used either way.

If classified is defined in taxa, then I will need it on CRAN for metacoder to work. The most important functions in metacoder require the information classified encapsulates. They were awkward to use before I adapted them to classified since they would require ~4 arguments that were all related to each other. sub setting data was a pain.

zachary-foster commented 8 years ago

If taxa is not ready for CRAN by the time I need it to be, then I can copy the relevant code from taxa into metacoder temporarily until taxa is on CRAN.

sckott commented 8 years ago

cool, okay

sckott commented 8 years ago

@zachary-foster Looked over the code. I like many of the ideas. Would you be okay with this:

zachary-foster commented 8 years ago

Thanks for looking it over!

Can we use R6 for the base classes? We'll likely have wrappers around those, so users probably won't be exposed to them - but I think this could be sufficiently complex that it may makes sense to use R6

Yes, I only used S3 to get some practice with it and because I could do get a proof of concept working quickly. Much of the code needs to be largely rewritten anyway in my opinion. I like the idea of insulating the user from the class system in case things need to be changed in the future and to preserve a traditional R feel for those that don’t like object oriented programming.

Looks like you've done NSE without lazyeval - would it be easier to just use it?

It probably would! I did not know about lazyeval. I followed Hadley’s "Advanced R" book; odd he didn’t mention his own package. It looks like lazyeval would be preferable to my novice attempts at NSE from scratch.

After having used the code in this PR in my metacoder package, I noticed a few things I would like to change in the internal workings, but the general idea has worked well (all the input/output objects in the demonstration on the homepage are classified objects). I have copied this PR code into metacoder so that it can be independent from taxa until things are working.

sckott commented 8 years ago

Yes, I only used S3...

cool cool

It probably would! I did not know about lazyeval. ...

great

After having used the code in this PR in my metacoder package, I noticed a few things I would like to change in the internal workings, but the general idea has worked well (all the input/output objects in the demonstration on the homepage are classified objects). I have copied this PR code into metacoder so that it can be independent from taxa until things are working.

Okay, sounds good