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

Implement automatic tool generation based on the source code of the tool #1263

Merged
merged 72 commits into from Sep 14, 2023

Conversation

Kulivox
Copy link
Contributor

@Kulivox Kulivox commented Aug 22, 2022

Hello,
A few months ago I had to implement a bunch of tool wrappers for an open source toolkit and realised how much time could be saved if there was an option of automatic generation of parts of the wrapper in tool_init. That lead me to develop PyGalGen, my take on the functionality of tool_init. I have shown the tool to @martenson, and he has agreed that migration of the part of the functionality that generates the wrapper file is worthwhile.

This pull request contains the prototype of the functionality. The generator is capable of extracting inputs and generating a command section for tools written in Python3 that use the ArgumentParser module.
There is still some work to be done, mainly in testing and documentation.

Current functionality:
Parser information can be extracted if the following conditions are met:

  • tool uses ArgumentParser
  • the parser init is in a single function

TODO:

  • add documentation

Example input: url
Example output: https://pastebin.com/v9z4ZMAD

Suggestions are welcome!

@Kulivox Kulivox marked this pull request as ready for review August 22, 2022 15:42
@mvdbeek mvdbeek requested a review from hexylena August 22, 2022 16:08
@bernt-matthias
Copy link
Contributor

Hi @Kulivox are you aware of https://github.com/hexylena/argparse2tool

@Kulivox
Copy link
Contributor Author

Kulivox commented Aug 23, 2022

Hi @Kulivox are you aware of https://github.com/hexylena/argparse2tool

Hi, no, I didn't know such project already exists
Good to know I guess :D

@hexylena
Copy link
Member

hexylena commented Sep 8, 2022

@bernt-matthias I met with @Kulivox @biomonika and discussed a bit how to proceed, what to do with argparse2tool. They discussed their methods, it seems a bit safer/cleaner than my fake argparse method and I'm happy to support this being in planemo, instead of a2t, and instead of attempting to merge the two as I think the two of us are the primary users. So we'll let them co-exist rather than spending the time to try and merge.

It would be nice to have a2t's functionality with both cwl + galaxy export, but I think that's not a priority, we can always direct people to use a2t instead if they need that functionality, just means I'll probably do less maintenance of it. @Kulivox's implement is more complete it seems, a2t is missing argparse command groups currently.

And TIL planemo has a text tool template inside, maybe that's a use case for our galaxyxml (but maybe that's also not worth the return on investment?)

@Kulivox would you be open to handling ('output', type=argparse.FileType('w')) as a tool output? I think that's the only major difference I see, and I'm pretty sure I'm the only person alive who uses it, but, I'd still like it. We also have some existing tool test cases, if you wanted to use those as a basis for the tests here.

@Kulivox
Copy link
Contributor Author

Kulivox commented Sep 11, 2022

@hexylena Sure, I'll add the type mapping in, and thanks; I think I'll also use the tool examples you provided to create tests

@Kulivox
Copy link
Contributor Author

Kulivox commented Sep 30, 2022

I'll be doing a final pass over the functionality of autpygen during the next few days, but I don't think I'll be changing much, maybe adding support for some very specific argument types and some cleanup of the output. The PR is IMHO ready for review, I have updated automatically generated docs, but I won't be adding custom docs until the functionality is finalised. Would it be possible for you @bernt-matthias to take a look at this PR? Or should I contact someone else?

@bernt-matthias
Copy link
Contributor

@Kulivox will you be at the EGD in Freiburg next week?

@Kulivox
Copy link
Contributor Author

Kulivox commented Sep 30, 2022

@bernt-matthias No, unfortunately

Copy link
Contributor

@bernt-matthias bernt-matthias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here some first comments. So far I have only checked the test case.

Wondering if https://github.com/hexylena/galaxyxml/ would be of any use. But I guess you already discussed this with @hexylena .

tests/data/autopygen/autopygen_end_to_end_test_case.py Outdated Show resolved Hide resolved
tests/data/autopygen/autopygen_end_to_end_sub_commands.txt Outdated Show resolved Hide resolved
tests/data/autopygen/autopygen_end_to_end_sub_inputs.xml Outdated Show resolved Hide resolved
tests/data/autopygen/autopygen_end_to_end_sub_inputs.xml Outdated Show resolved Hide resolved
tests/data/autopygen/autopygen_end_to_end_test_case.py Outdated Show resolved Hide resolved
tests/data/autopygen/autopygen_end_to_end_test_case.py Outdated Show resolved Hide resolved
tests/data/autopygen/autopygen_generated_inputs.txt Outdated Show resolved Hide resolved
@bernt-matthias
Copy link
Contributor

Hey @Kulivox would you be available for a VC on Thursday or Friday? I guess the easiest for a review would be if you could walk us through your code.

Maybe @hexylena and @nsoranzo would like to join?

@hexylena
Copy link
Member

hexylena commented Oct 4, 2022 via email

@Kulivox
Copy link
Contributor Author

Kulivox commented Oct 4, 2022

Hi, I should be available on Friday after 4 pm CET. Does that work for you too? @bernt-matthias @hexylena

@bernt-matthias
Copy link
Contributor

Hi @Kulivox I will be on the train from approx 2pm. So unfortunately, 4pm won't work for me.

@Kulivox
Copy link
Contributor Author

Kulivox commented Oct 5, 2022

Ah, that is unfortunate. I'll be at work from 7-15 on Friday, so I can't do the call earlier. What about some later date? I should be available anytime during the weekend and after 4pm on weekdays @bernt-matthias

@bernt-matthias
Copy link
Contributor

Hi @Kulivox Due to vacation I will only have time from Nov 7 :(. But maybe @hexylena and @nsoranzo could help earlier...?

@Kulivox
Copy link
Contributor Author

Kulivox commented Oct 7, 2022

Hi, @hexylena @nsoranzo, would it be possible to arrange a VC where we take a look at the PR as @bernt-matthias suggested? I should be available any day apart from Tuesday.

@biomonika
Copy link

Hi @bernt-matthias , I hope you have had a great time away. If you're up for it, could we please continue the review? Thanks a lot for your help!

@bernt-matthias
Copy link
Contributor

Hi @Kulivox and @hexylena could you suggest a date / dates during next week? Currently I'm available except for Wednesday Morning (Berlin time).

@Kulivox
Copy link
Contributor Author

Kulivox commented Nov 8, 2022

@bernt-matthias I am free on Monday afternoon (16:30 + CET), Tuesday evening (18:00+) and then on any day from 7-9 AM CET and from 16:15-23:59 CET

@bernt-matthias
Copy link
Contributor

Monday afternoon (16:30 + CET) would be cool for me.

@hexylena
Copy link
Member

hexylena commented Nov 9, 2022 via email

@Kulivox
Copy link
Contributor Author

Kulivox commented Nov 12, 2022

@hexylena @bernt-matthias Alright then, I'll set up a meeting starting at 17:00 because I might not be ready at 16:30 (train journey home)

@bernt-matthias
Copy link
Contributor

Wonderful.

@bernt-matthias
Copy link
Contributor

Currently testing another tool. If I find some things that might be improved I will add them here:

If there is a command line boolean flag: --flag it would be great to define it with truevalue="--flag" falsevalue="--flag" checked="true/false" (checked depending on the default) .. the value attribute should be omitted.

Use it in the command just as $flag.

Or wouldn't this work for:

    parser.add_argument('--flag', metavar='BOOL', type=bool, default=true)

?

@bernt-matthias
Copy link
Contributor

I guess we should merge this soon. It's really good and usable in my opinion. We can still improve it and fix bugs later on... or what do you think @mvdbeek @nsoranzo and @hexylena ?

@bernt-matthias bernt-matthias changed the title [WIP] Implement automatic tool generation based on the source code of the tool Implement automatic tool generation based on the source code of the tool Sep 1, 2023
@bernt-matthias
Copy link
Contributor

This is good to go in my opinion. Not sure about docs: Obviously a good idea, but I don't know where it would fit best and what the extent/form should be.

@mvdbeek
Copy link
Member

mvdbeek commented Sep 1, 2023

Yes, at least a small example would be nice, maybe in writing_standalone.rst ? This is nicely isolated and looking good.

@mvdbeek mvdbeek merged commit 6d6cb43 into galaxyproject:master Sep 14, 2023
14 checks passed
@mvdbeek
Copy link
Member

mvdbeek commented Sep 14, 2023

Thanks for your work @Kulivox! I've pushed a new planemo version to https://pypi.org/project/planemo/0.75.11/

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.

None yet

6 participants