Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add py requirement and actually output #44

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

bernt-matthias
Copy link

No description provided.

@bebatut
Copy link
Member

bebatut commented Jan 25, 2021

Thanks @bernt-matthias for this update.
I just realized I already made the changes (but forgot to push. fixed now) and updated the wrapper on the ToolShed in September. Sorry for that.

I wander. Did you get an error with the new version?

@bernt-matthias
Copy link
Author

I wander. Did you get an error with the new version?

One of my users reported an error with v 0.1.0

  File "/gpfs1/data/galaxy_server/galaxy/shed_tools/toolshed.g2.bx.psu.edu/repos/bebatut/combine_metaphlan2_humann2/31394a0c0242/combine_metaphlan2_humann2/combine_metaphlan2_humann2.py", line 87
    print "no", genus, "found in", args.metaphlan2_file
             ^
SyntaxError: Missing parentheses in call to 'print'. Did you mean print("no", genus, "found in", args.metaphlan2_file)?

So in this PR only quoting the python tool directory remains.

@bernt-matthias
Copy link
Author

Btw. shall we setup IUC style github actions on this repo?

@bernt-matthias
Copy link
Author

The version in the toolshed seems to still miss the python2 requirement.

@bebatut
Copy link
Member

bebatut commented Jan 25, 2021

It should be working with Python 3 now

@bebatut
Copy link
Member

bebatut commented Jan 25, 2021

Btw. shall we setup IUC style github actions on this repo?

Maybe. But I have no idea how it works (I was in parental leave the whole year). I could maybe also move these wrappers to IUC

@bernt-matthias
Copy link
Author

It should be working with Python 3 now

One still needs the requirement, e.g. for cases where the system has python 2. Let me just add it here.

Just understood why my user used the old version: he is using the GTN workflow which is probably using 0.1.0

Maybe. But I have no idea how it works (I was in parental leave the whole year). I could maybe also move these wrappers to IUC

The change should be pretty simple. I could create a PR, and you would need to add the TS API keys in the settings.

  • For pull requests it also lints python and R scripts in addition to tool linting and testing (furthermore tests are executed using containerized testing)
  • Furthermore a weekly test of all tools is executed.

But moving to IUC is also a good idea but probably more work. Up to you.

@bernt-matthias
Copy link
Author

Finished from my side. Should I bump the tool version?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants