Skip to content

Conversation

@cthoyt
Copy link
Contributor

@cthoyt cthoyt commented Oct 2, 2020

Reference Issues/PRs

Closes #152

What does this implement/fix? Explain your changes.

This PR does several things to improve the CLI:

  1. Replaces docopt with click (it's much easier to work with)
  2. puts the selene_cli.py inside the package as cli.py
  3. exposes the CLI using __main__.py so it can be run with python -m selene_sdk
  4. exposes the CLI using the entry_points kwarg in setup.py so it can be run in the shell as selene_sdk
  5. Improve the documentation related to the CLI to make this clear to anyone reading it later

Even though it switched from docopt to click, and also the location moved, everything should continue working exactly the same way. Click auto-generates the documentation for the CLI and I posit is better than docopt, but if you don't agree, I could also change it back.

Happy hacktoberfest / greetings from Germany!

What testing did you do to verify the changes in this PR?

It still does the same as before, now with just a slicker interface!

Closes FunctionLab#152

This PR does several things to the CLI:
1. replaces docopt with click (it's much easier to work with)
2. puts the selene_cli.py inside the package as cli.py
3. exposes the CLI using __main__.py so it can be run with python -m selene_sdk
4. exposes the CLI using the entry_points kwarg in setup.py so it can be run in the bash as selene_sdk
5. Improve the documentation related to the CLI to make this clear to anyone reading it later

Happy hacktoberfest / greetings from Germany!
@kathyxchen
Copy link
Collaborator

@cthoyt Thank you so much for submitting this PR! I completely agree with the comments you made in the corresponding issue - it's a change we hadn't prioritized but will make the user experience of the library so much better, this is really awesome. :) I'll be sure to review your PR sometime this week.

Copy link
Collaborator

@kathyxchen kathyxchen left a comment

Choose a reason for hiding this comment

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

Thank you again for this contribution! I have one minor ask - can you add click (with whatever version you are using) to the selene-gpu.yml and selene-cpu.yml conda environment files? Just because some people will only use Selene locally (i.e. without setup.py install or conda install). I think we can also remove selene-gpu-snapshot.yml as well since the other two files are more usable. LGTM otherwise. :) I'll approve and push after those changes are made!

@cthoyt
Copy link
Contributor Author

cthoyt commented Oct 7, 2020

Done in cthoyt-forks-and-packages@58cf55f, cthoyt-forks-and-packages@e732528, and cthoyt-forks-and-packages@1aff5e2. Let me know if that's sufficient

Copy link
Collaborator

@kathyxchen kathyxchen left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you so much for your contribution!

@kathyxchen kathyxchen merged commit 2150182 into FunctionLab:master Oct 7, 2020
@cthoyt cthoyt deleted the package-cli branch October 7, 2020 18:19
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.

Include CLI within the package

2 participants