GaNDLF icon indicating copy to clipboard operation
GaNDLF copied to clipboard

Convert main scripts to console entry points

Open sarthakpati opened this issue 1 year ago • 13 comments

Is your feature request related to a problem? Please describe. Currently, GaNDLF's main interactive components are based on standalone scripts, which are an older way of defining python packages.

Describe the solution you'd like Converting them to dedicated functions with console entry points would be better (a good read on why is here), and more compatible with future changes in python.

Describe alternatives you've considered N.A.

Additional context More details in: https://python-packaging.readthedocs.io/en/latest/command-line-scripts.html

sarthakpati avatar Apr 03 '23 13:04 sarthakpati

Stale issue message

github-actions[bot] avatar Jun 02 '23 19:06 github-actions[bot]

@sarthakpati - Can i take this issue up or work on a bit on it? I have already signed the form for github participation and I am in the working group

sunny1401 avatar Jul 21 '23 19:07 sunny1401

@sarthakpati - Can i take this issue up or work on a bit on it? I have already signed the form for github participation and I am in the working group

Thank you so much, and welcome! I have assigned this issue to you.

sarthakpati avatar Jul 21 '23 19:07 sarthakpati

@sarthakpati - I have a doubt (sorry). I have finished going through the file structure etc.. I have one question - for example for GaNDLF.anonymize - I found this file. Is the intent of this ticket to just move/copy this code to command_line.py within anonymize/ and update the setup.py as entry_points={ "console_scripts": ["gandlf_anonymizer=GANDLF.anonymize:.console.main"] }

and then do this for all the interactive components. I have something for PR but I want to make sure I am on the right track before raising a PR

sunny1401 avatar Jul 29 '23 19:07 sunny1401

@sunny1401 - that's a great question. I personally feel it might be better for us to keep the algorithmic/functional components of the code separate from the portions of the code that deal with interfaces (which are current command-line only). Hence, perhaps putting this under GANDLF.cli might be better, and then updating the setup.py to:

entry_points={
    "console_scripts": [
        ...
        "gandlf_anonymizer=GANDLF.cli.anonymize:main",
        ...
    ]
}

This can similarly be extended for other such features that require command line interfaces, such as preprocessing:

entry_points={
    "console_scripts": [
        ...
        "gandlf_anonymizer=GANDLF.cli.anonymize:main",
        "gandlf_anonymizer=GANDLF.cli.preprocess_and_save:main",
        ...
    ]
}

And so on. This can then be extended for graphical interfaces (if and when needed). What do you think?

sarthakpati avatar Jul 30 '23 01:07 sarthakpati

Can you suggest a way to ensure the new code gets tested (currently, the cli scripts are getting skipped from code coverage)? The only thing that is coming to my mind is to change some of the existing tests to use the new interface(s). What do you think?

sarthakpati avatar Jul 31 '23 13:07 sarthakpati

Can you suggest a way to ensure the new code gets tested (currently, the cli scripts are getting skipped from code coverage)? The only thing that is coming to my mind is to change some of the existing tests to use the new interface(s). What do you think?

I could add additional test and mock out sys params passing to test end to end functionality for the cli module?

sunny1401 avatar Jul 31 '23 15:07 sunny1401

Can you suggest a way to ensure the new code gets tested (currently, the cli scripts are getting skipped from code coverage)? The only thing that is coming to my mind is to change some of the existing tests to use the new interface(s). What do you think?

I could add additional test and mock out sys params passing to test end to end functionality for the cli module?

Sure, let's see how much longer these tests take. Perhaps we can find a way to combine it with some of the others.

sarthakpati avatar Jul 31 '23 15:07 sarthakpati

Stale issue message

github-actions[bot] avatar Sep 29 '23 19:09 github-actions[bot]

Stale issue message

github-actions[bot] avatar Nov 29 '23 19:11 github-actions[bot]

Stale issue message

github-actions[bot] avatar Jan 29 '24 19:01 github-actions[bot]

The "standard" way of cli interface is usually a cli command with sub-commands, like gandlf anonymizer --params. What do you think if we support such an cli app / entrypoint? We may both support existing entrypoints (as you propose, gandlf_anonymizer) for backward compatibility, as well as keep just one cli app.

VukW avatar Feb 26 '24 07:02 VukW

That sounds awesome, thanks! 😄

If you are fine to work on this, then I will assign this to you, and we can proceed with your proposal. It would require us to update the documentation as well. But I think it would be a pretty big improvement WRT overall usability.

sarthakpati avatar Feb 26 '24 14:02 sarthakpati

Fixed in #818

sarthakpati avatar Apr 26 '24 13:04 sarthakpati