Code review #24

Hi and thanks for the nice package. Before moving to BioJulia, I took a look over your code and found a bunch of issues you might want to address:

KEGG itself is for academic users only. I think you should either

• Feature this prominently in the README or the docs of exported functions, or
• License the package to only academic users

You can't use batching with kegg_ddi. It means some combinations (of drugs in different batches) are not tested.

Where possible, specify the return type in the documentation signature.
Some functions do not describe what they return.

In kegg_conv, it is confusing that timeout refers to something other than a normal request timeout. It seems to be an interval between requests. I suggest renaming to another keyword. This is also present in other functions.

The code is inconsistent as to what KeggTupleList means. E.g. conv_parser returns two columns in the .data vector, whereas kegg_conv returns a vector of rows.
I think you should document the type more precisely. With comments, specify what each field means, then check all functions conform to that

chunk_vector already exists as collect(Iterators.partition(vec, chunk_size)). The existing implementation returns views, which might be preferable from a performance standpoint.

The examples in the docstring should be tested. See documenter for how to do this. For example, the documentation of kegg_conv calls a nonexistent function.

No structs are public or exported. I recommend making the types of all values that are returned from functions public. Also, RequestError should probably be public.

The documentation of request promises that a RequestError is returned on error. But HTTP can also throw exceptions so that is not true.

In kegg_ddi, repeated string concatenation is quadratic. Use a buffer to store strings, then call join at the end.

In kegg_list, there is no guarantee that a read with readavailable reads whole lines. Hence, the following eachline could read partial lines and corrupt data. Since you store all the data anyway, you might as well just read in all the data at once.

Base using SomePackage is not a good idea in registered packages, because if any identifier conflicts with a future export from Base, you can get a conflict. Instead, use the specific names you use from packages: using SomePackage: some_name, OtherName.

The docstring for kegg_list makes no sense - both kegg_list(brite::String, option::String) and kegg_list(genome::String, option::String) refer to the same signature. Should option be replaced with query_type?

If request is not inteded for direct use, why is it exported?

Consider marking const fields of mutable structs const.

There is no compat for Test or Runic - okay since they're test dependencies, but still a good idea.

CodeCov does not seem to work - at least the coverage is "unknown" on the README badge, and clicking the badge leads to a dead link.

I believe colnames in conv_parser is wrong - KEGG orders sources first, then target.