search

harmslab / topiary

Python framework for doing ancestral sequence reconstruction
MIT License
31 stars 7 forks source link

Add support for Entrez.api_key #30

Closed odcambc closed 1 year ago

odcambc commented 1 year ago

Thanks for the great tool! It would be nice to allow users who have an API key to use it to increase access rates - one can just set Entrez.api_key in topiary/ncbi/init.py along with Entrez.email. I'm not sure where that would best be implemented by the user in topiary, however.

harmsm commented 1 year ago

This is a great idea. I think the most reasonable way to do it from a user perspective would be to have the user set an environment variable. I generally don’t like using environment variables as they make set up harder, but if someone has an API key, I suspect they’re an advanced user anyway, so setting an environment variable won’t be a barrier.

I think some other tools use NCBI_API_KEY already, so maybe that’s a good choice? Sound reasonable from your perspective?

Thanks for your input!

Mike

On Jan 24, 2023, at 4:23 PM, Chris Macdonald @.***> wrote:

Thanks for the great tool! It would be nice to allow users who have an API key https://support.nlm.nih.gov/knowledgebase/article/KA-05317/en-us to use it to increase access rates - one can just set Entrez.api_key in topiary/ncbi/init.py along with Entrez.email. I'm not sure where that would best be implemented by the user in topiary, however.

— Reply to this email directly, view it on GitHub https://github.com/harmslab/topiary/issues/30, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABFZA6RXCHT6KB5TWEAO323WUBXB7ANCNFSM6AAAAAAUFW67EA. You are receiving this because you are subscribed to this thread.

odcambc commented 1 year ago

That sounds like an ideal option - I just made a quick PR to implement that. I'm not sure if it would be best to alert the user if the api key is being set or not, but that would be simple to add to the try block there.

harmsm commented 1 year ago

Thanks for PR. I just made a few tweaks to your PR and merged as (https://github.com/harmslab/topiary/pull/32).Thanks for your help with this!

harmsm commented 1 year ago

Nice. PR looks good. The change will have to be a bit more substantial because topiary also enforces the request limit against NCBI servers. It will require a couple of tweaks