search

ropensci / popler

The R package to browse and query the popler database
https://docs.ropensci.org/popler
MIT License
8 stars 7 forks source link

Do we need the `get_data` class? #49

Closed AldoCompagnoni closed 7 years ago

AldoCompagnoni commented 7 years ago

The line: d <- get_data(proj_metadata_key ==1) Returns an object of class popler, get_data, and data.frame.

I do not think we need a class for get_data - but there is something I might be missing!

bochocki commented 7 years ago

Having a get_data class helps a lot for the summary functions of dictionary and browse.

There is a function called rebrowse() that takes an object of popler class as an input and returns the equivalent browse() call with trim=FALSE, full_tbl=TRUE, but it needs to do different things depending on whether the popler object is a get_data object or a browse object.

By making separate classes, we can create a rebrowse function with different methods (i.e., rebrowse.get_data() and rebrowse.browse(). This makes it easier to change code (and debug) each method independently. It also shouldn't interfere with anything, since most functions will not have methods for browse or get_data classes.

We could get rid of the classes, but I'm not sure we gain much and we'd need to re-write the rebrowse function using something a bit hackier... I was originally looking for column names that were only returned by either browse() or get_data().

bochocki commented 7 years ago

Sorry, I wrote that first sentence without thinking. rebrowse() is used in the summary function for browse (what used to be called "fancy browse") and the citation function.

AldoCompagnoni commented 7 years ago

I love it! Issue closed!