AAHash

[jwcodee]

This PR contains aaHash and associated changes to integrate aaHash into the btllib environment.

Non-exhaustive list of changes

• Enable seq_reader and seq_writer to read and write amino acid sequences
• inline nte function to accelerate generate multiple hashes
• expanded unit tests

I intend to make a release v1.6.0 PR as soon as this PR is approved. As we are trying to submit aaHash as soon as possible, there are some outstanding items to be implemented

TBD in 1.6.X:

• python wrapper for seedAAHash

[lcoombe]

Thanks for all this work Johnathan and Parham!

A couple of additional comments in addition to the couple in the review:

• Have you updated the docs?
• Could you add tests for the python wrappers? I think it would be good for us to add those upfront when adding new btllib code moving forward

[jwcodee]

Thanks for all this work Johnathan and Parham!

A couple of additional comments in addition to the couple in the review:

• Have you updated the docs?
• Could you add tests for the python wrappers? I think it would be good for us to add those upfront when adding new btllib code moving forward

I haven't updated the docs. The readme said to run ninja docs before the release itself.

Sure I can add python tests.

[parham-k]

It's ok to generate docs here since we're releasing straight after merging

[lcoombe]

It's ok to generate docs here since we're releasing straight after merging

I agree - also think it's best to keep the master branch code in sync with the docs

[jwcodee]

Updated docs generated with conda version of doxygen (1.9.6 (c4c52d9716f313ded2deb5e7cdbc02bbcf389ad3*)). Seems like some docs were generated with a previous version.

[parham-k]

Thanks for your work @jwcodee! Please run ninja wrap with the latest version of SWIG, and add more documentation and checks about the seed strings. Also, thanks @lcoombe and @vlad0x00 for your suggestions.

[parham-k]

Thanks for the changes @jwcodee. I don't want to be blocking this from being merged while I'm off, so I'll approve for now. Feel free to merge whenever everything's working fine.

[jwcodee]

Based on Vlad's suggest, I used constexpr inline instead of static const but this feature requires c++17. When c++17 is enabled, clang-tidy flags new errors. Simply changing the instances unique_ptr to make_unique generates new errors that are not easily fixed so I opted to use nolint.

[parham-k]

When c++17 is enabled, clang-tidy flags new errors.

I also encountered these errors when I wanted to integrate argparse, which requires C++17, into our recipes. Created #90 to track this in a future PR.

[jwcodee]

Murathan has resolved the python wrapper issue. The problem seems to be that the string data got cleared by some interaction. Murathan made a new private member to store a copy and that alleviates the issue. I am testing if it will increase ram substantially in the RAM test.

[parham-k]

If this does increase the RAM usage, we should consider switching our std::string&s to std::string_views (bcgsc/ntHash#45).

[jwcodee]

If this does increase the RAM usage, we should consider switching our std::string&s to std::string_views (bcgsc/ntHash#45).

That worked. I am now using std::string_view in place of const std::string& and reverted Murathan's proposed suggestion.