KwanLab / Autometa

Autometa: Automated Extraction of Genomes from Shotgun Metagenomes
https://autometa.readthedocs.io
Other
40 stars 15 forks source link

autometa-taxonomy-lca return TypeError without specifying --cache #246

Closed chtsai0105 closed 2 years ago

chtsai0105 commented 2 years ago

Current Behavior

Running autometa-taxonomy-lca without specifying --cache would return a TypeError.

I think the following lines might cause the issue. https://github.com/KwanLab/Autometa/blob/ad0364a505bb188a3bbcd9139ecf09895572127c/autometa/taxonomy/lca.py#L77-L85 Without specifying --cache, it would become a NoneType and would cause a TypeError when passing to the os.path.exists(). Maybe can assign a default path for the cache dir to fix it.

Steps to Reproduce

autometa-taxonomy-lca --blast data/blastp.tsv --dbdir ~/home/bigdata/autometa/databases/ncbi --lca-output data/lca.tsv --verbose

[03/09/2022 11:00:48 AM DEBUG] autometa.taxonomy.ncbi: Processing nodes from /rhome/ctsai085/bigdata/autometa/databases/ncbi/nodes.dmp
[03/09/2022 11:01:04 AM DEBUG] autometa.taxonomy.ncbi: nodes loaded
[03/09/2022 11:01:04 AM DEBUG] autometa.taxonomy.ncbi: Processing names from /rhome/ctsai085/bigdata/autometa/databases/ncbi/names.dmp
[03/09/2022 11:01:19 AM DEBUG] autometa.taxonomy.ncbi: names loaded
[03/09/2022 11:01:19 AM DEBUG] autometa.taxonomy.ncbi: Processing nodes from /rhome/ctsai085/bigdata/autometa/databases/ncbi/merged.dmp
[03/09/2022 11:01:20 AM DEBUG] autometa.taxonomy.ncbi: merged loaded
Traceback (most recent call last):
  File "/rhome/ctsai085/.conda/envs/metagenome/bin/autometa-taxonomy-lca", line 10, in <module>
    sys.exit(main())
  File "/rhome/ctsai085/.conda/envs/metagenome/lib/python3.8/site-packages/autometa/taxonomy/lca.py", line 775, in main
    lca = LCA(dbdir=args.dbdir, verbose=args.verbose, cache=args.cache)
  File "/rhome/ctsai085/.conda/envs/metagenome/lib/python3.8/site-packages/autometa/taxonomy/lca.py", line 86, in __init__
    self.tour_fp = os.path.join(self.cache, "tour.pkl.gz")
  File "/rhome/ctsai085/.conda/envs/metagenome/lib/python3.8/posixpath.py", line 76, in join
    a = os.fspath(a)
TypeError: expected str, bytes or os.PathLike object, not NoneType

Expected Behavior

Should create a default path for cache dir when it is not assigned.

Environment Information

autometa-configure --dryrun --debug

I installed autometa from conda and do not have autometa-configure... I tried autometa-config --dryrun --debug but it do not have such arguments.

evanroyrees commented 2 years ago

Hello @chtsai0105, thank you again for bringing another hiccup to our attention! If you would like to submit a pull request, an easy fix would be to change the default from None to an empty string , e.g. "". For now, to save space, we do not require the user create a cache for the LCA process.

I can submit a pull request to fix this soon (or you are welcome to contribute, if you are not busy). Thanks again for pointing this out.


I installed autometa from conda and do not have autometa-configure... I tried autometa-config --dryrun --debug but it do not have such arguments.

Thank you as well for pointing this out, we need to update our new issue template 😅 👍

chtsai0105 commented 2 years ago

Hi @WiscEvan, I've review the codes again and made some fixes:

https://github.com/KwanLab/Autometa/blob/ad0364a505bb188a3bbcd9139ecf09895572127c/autometa/taxonomy/lca.py#L775 https://github.com/KwanLab/Autometa/blob/ad0364a505bb188a3bbcd9139ecf09895572127c/autometa/taxonomy/lca.py#L77 First, because the args.cache have been passed into the LCA class in the main function. So it would still remain to be None if users are not specifying, even we set the default as "" inside LCA.

https://github.com/KwanLab/Autometa/blob/ad0364a505bb188a3bbcd9139ecf09895572127c/autometa/taxonomy/lca.py#L83-L85 Second, the if statement here is not what cause the error. Since the statement would be skipped when the cache is None.

https://github.com/KwanLab/Autometa/blob/ad0364a505bb188a3bbcd9139ecf09895572127c/autometa/taxonomy/lca.py#L86-L93 What really cause the error is these lines, which trying to join the string with the NoneType. So my fix is to move all the os.path.join() codes under the if statement and leave the rest of them out there. But still need to evaluate whether the change would cause any side-effect.

evanroyrees commented 2 years ago

First of all, thanks for contributing a PR! 🥳 🎉

Unfortunately, these changes will break some of the other methods in the LCA class. I will submit suggested changes on your PR so we can get this merged in 👍