grunwaldlab / metacoder

Parsing, Manipulation, and Visualization of Metabarcoding/Taxonomic data
http://grunwaldlab.github.io/metacoder_documentation
Other
135 stars 28 forks source link

Suggestion: make internal functions truely internal #46

Closed zkamvar closed 8 years ago

zkamvar commented 8 years ago

When I look at the index for metacoder, there are a lot of functions that don't seem to be immediately of use to the user, such as resizingTextGrob(). Additionally, I notice that there are unexported functions documented such as verify_color_range().

These create a lot of clutter in the index for metacoder and might confuse users. I have a couple of suggestions to de-clutter these.

Exported internal functions

Don't export them. You can change/add/remove internal functions to your heart's desire, but an exported function requires a version update and has the potential to break a user's workflow.

Internal documentation

Documenting internal functions is a fantastic idea, but it should not be displayed in the index since that becomes the table of contents for the user manual. Instead, I recommend to add @keywords internal to your roxygen directives for the unexported functions. This will still create the documentation for those who need it, but will hide it from those who don't.

zachary-foster commented 8 years ago

Good ideas. I appreciate you looking through the package. In regards to resizingTextGrob, I think it needs to be exported since the grid package must have access to it. resizingTextGrob is an odd case, for the others there are probably some that can un-exported.

zkamvar commented 8 years ago

In regards to resizingTextGrob, I think it needs to be exported since the grid package must have access to it.

That is a very interesting conundrum. Usually anything that happens inside your package has access to anything within your packages internal namespace. What happens when you don't export it?

zachary-foster commented 8 years ago

I am rusty on the details, but I think resizingTextGrob extends the grid package in some way. Its not my package that need access to grid, but grid that needs access to my package. And, its not a direct reference to my package that could be done with the ::: accessor for internal functions; its something name-based or class based. I don’t think that gird knows about resizingTextGrob unless it is exported.

zkamvar commented 8 years ago

I feel a disturbance in the force...

zachary-foster commented 8 years ago

Haha. Hopefully its not that bad...