Refactor Betydb functions to support new API

[infotroph]

As seen in #77:

• Remove betydb_GET2 if redundant
  • Done. Note that this means betydb_GET now returns a dataframe for multi-item queries and a (possibly nested) list for the detail view on a single ID.
• target API by version
  • I implemented this in betydb_GET (which handles the JSON), not betydb_http (which returns a single text string and only cares about API version for include strings)
• make generic functions for table queries (Merge BETYdb table queries into a single generic function #26) and single records
  • add betydb_query()
  • add betydb_record()
  • make other query functions use betydb_query under the hood
• Update documentation
  • I think this is mostly done; feedback, please.
• Allow unchecked data (Add option to retrieve 'unchecked' data from BETYdb #52). Already works in betydb_query, need to document and make work in singular functions.

[infotroph]

Not sure how I managed a merge conflict in an autogenerated file, but should be fixed now

[infotroph]

@dlebauer As currently written, I'm defaulting to the v0 API so anyone relying on previous versions can keep their old behavior, but that means regex searches always need to set api_version="beta" or they fail quietly. Switch default to api_version="beta"? Set options("betydb_api_version")?

[dlebauer]

If you can pass API version, URL, and key in options() that would make the code easier to read. I've never written a function that used this approach but it would be nice if it wasn't necessary to explicitly pass these with each function call.

An alternative would be to create a list of connection parameters and pass that instead of having separate arguments.

The options approach seems more elegant but the list approach seems easier to implement and maintain (mostly because it is more straightforward). What do you think?

@sckott any guidance wrt best practices/ ropensci convention?

[infotroph]

Without presuming anything on ropensci convention, I'll note I'm usually suspicious of options as 'basically globals by another name', but then again this likely is a global preference -- the user probably wants all queries in any one session to go to the same API on the same server.

How about betydb_foo(..., api_version=getOption("bety_api_version", "v0"), betyurl=getOption("bety_url", "https://www.betydb.org"), and so on? Then the defaults are sane and visible to everyone, and options are respected if set but we don't clutter up the options list for users who don't set them.

[dlebauer]

That seems sensible, but is this preferable to passing a list connection = list(url, key, version) ?

[infotroph]

Probably equal, unless one's more in the ropensci spirit.

[sckott]

at a meeting reply soon

[dlebauer]

@infotroph the failed build is due to the function ncbi_byname (emitting a warning that is treated as error in the build) and is also causing the master branch to fail

WARNING
Undocumented arguments in documentation object 'ncbi_byname'

[dlebauer]

@infotroph I think this looks good.

@sckott could you merge this? I'd like to use it in a workshop Friday, and I'd prefer to use install_github('ropensci/traits') than install_github('infotroph/traits@betydb_refactor') ... if possible.

[sckott]

looks good, seems like there may be stuff at top of issue #82 (comment) that isn't done yet, open any new issues if needed

[infotroph]

Thanks! For the record, here are issues for the checkboxes not ticked at merge time:

• "make other query functions use betydb_query under the hood" on hold pending decision on the utility of the other functions (Use cases for non-ID search fields in singular betydb_* functions #84)
• "allow unchecked data" works but needs documentation (Add option to retrieve 'unchecked' data from BETYdb #52)
• "update documentation" is otherwise done.

[sckott]

thank