ksahlin / ultra

Long-read splice alignment with high accuracy
60 stars 10 forks source link

Non-absolute paths don't resolve #8

Closed lassefolkersen closed 2 years ago

lassefolkersen commented 2 years ago

Hi! We're trying to implement this package in nf-core modules, and it's stumbling on paths not being able to resolve, unless absolute. So basically, one have to do \$(pwd)/$gtf instead of just e.g. ./$gtf. Makes it a little less clean to implement, any chance you could help with making the next version robust to this?

It seems that this place is (one of) the offending lines. I'm thinking one could put in the \$(pwd)/$gtf logic inside the repo here, to keep it clean in the nf-core implementation. Would also help with general portability of code. I'm thinking just adding os.path.abspath around the path calls would be enough:

https://github.com/ksahlin/ultra/blob/a6b6fb016cabcbf4ba482ddddb120a910b651b37/uLTRA#L63-L64

ksahlin commented 2 years ago

Hi @lassefolkersen

After looking into this, I think it would suffice to change within uLTRA lines 61-69 to

    elif not args.disable_infer:
        db = gffutils.create_db(args.gtf, dbfn=database, force=True, keep_order=True, merge_strategy='merge', 
                                sort_attribute_values=True)
        db = gffutils.FeatureDB(database, keep_order=True)
    else:
        db = gffutils.create_db(args.gtf, dbfn=database, force=True, keep_order=True, merge_strategy='merge', 
                                sort_attribute_values=True, disable_infer_genes=True, disable_infer_transcripts=True)

That is, removing the unnecessary line fn = gffutils.example_filename(args.gtf).

This would solve the issue of requiring an absolute path within the program and it doesn't require any change in the call to the program.

Would this be good?

lassefolkersen commented 2 years ago

I think so - but the verdict really is if it can run the nf-core unit tests. Not whether I like it or not. I can help with running some tests if you'd like. (or just pull down the test-module yourself and maybe replace the containers line to something local where you have your changes in)

ksahlin commented 2 years ago

I implemented the change proposed above and I uploaded it as a new version of uLTRA to PyPI (v0.0.4.1 https://pypi.org/project/ultra-bioinformatics/). Let me know if this resolves the issue.

sguizard commented 2 years ago

Tested and validated :+1: Thanks for the patch! The bioconda recipe has been updated. The nf-core update is in process.

ksahlin commented 2 years ago

Super, thanks! I'll close this then.