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

Added a CLI #38

Merged
merged 10 commits into from Apr 12, 2021
Merged

Added a CLI #38

merged 10 commits into from Apr 12, 2021

Conversation

quantum9Innovation
Copy link
Member

Basic CLI Structure Implemented


New Feature

This PR adds a basic built-in CLI to epispot nightly that comes preinstalled with setup.py. The keyword to initiate it is epispot followed by a command. Currently the only available command is about. The shell uses Fire and termcolor as dependencies although it can run without termcolor by suppressing color (see the __color__ flag). To test the shell, run:

$ epispot about
>>> Epispot was invoked via the epispot CLI
>>> Path: /home/user/anaconda3/lib/python3.8/site-packages/epispot_nightly-2.1.1.1-py3.8.egg/EGG-INFO/scripts
>>> Version: shell-v0.1.0-alpha epispot-v2.1.1.1
>>> Color output enabled (True/False): True

Known Issues

  • Coverage does not work on CLI scripts.
  • Planning on adding an enhancement to run models from the CLI; will require the addition of automatic
    parameter substitutions that will be implemented in another branch.

Code Breakdown

  • .coveragerc
    • Added bin/ as a directory however still not implemented in codecov reports (issue)
  • .github/workflows
    • Added to support build tests for CLI
    • coverage.yml
      • Added more verbosity for issue solving related to code coverage reporting
  • CHANGELOG.md
    • Added a CHANGELOG to track progress on the nightly package
  • README.md
    • Removed Release Notes on README.md to create CHANGELOG.md
  • bin/requirements.txt
    • Added separate CLI requirements (Fire, termcolor (optional))
  • requirements.txt
    • Removed unnecessary requirements (sphinx)
  • setup.py
    • Added scripts indicator to add epispot to the command line

Additional Notes

Similar to branch #cli
Ideally merge when:

  • Code and implementation has been thoroughly reviewed
  • New workflows have been approved by @Quantalabs 's review
    Ignoring:
  • LGTM, CodeFactor, and other code quality checks (except built-in GitHub CodeQL)
    • Currently transitioning to Deepsource

- Added *very* basic epispot CLI functionality
  - See `bin/requirements.txt` for CLI requirements
- Fire up a terminal and enter
```shell
$ epispot about
```
Added changelog separate from README.md for nightly package
 - Added bin/ to .coveragerc
 - Renamed bin/epispot to bin/epispot.py in workflow so that it is tracked by coverage.py
Invoked via test_invoke.py
Found in bin/requirements.txt
@quantum9Innovation quantum9Innovation added duplicate This issue or pull request already exists feat 🚀 New feature or request low-priority 🔽 Will be worked on later labels Apr 11, 2021
@codecov
Copy link

codecov bot commented Apr 11, 2021

Codecov Report

Merging #38 (73b216c) into nightly (6ba3f5e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           nightly      #38   +/-   ##
========================================
  Coverage    98.27%   98.27%           
========================================
  Files            6        6           
  Lines          464      464           
========================================
  Hits           456      456           
  Misses           8        8           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6ba3f5e...73b216c. Read the comment docs.

@quantum9Innovation
Copy link
Member Author

Ready to merge after workflow approvals from @Quantalabs

@lgtm-com
Copy link

lgtm-com bot commented Apr 11, 2021

This pull request introduces 3 alerts when merging 200f5d7 into 6ba3f5e - view on LGTM.com

new alerts:

  • 2 for Except block handles 'BaseException'
  • 1 for Unused import

@quantum9Innovation quantum9Innovation added high-priority 🔼 This is important and removed low-priority 🔽 Will be worked on later labels Apr 11, 2021
@quantum9Innovation quantum9Innovation added this to In progress in Development v2 Apr 12, 2021
.github/workflows/build.yml Outdated Show resolved Hide resolved

if __name__ == '__main__':
fire.Fire(Shell(
color=__color__
Copy link
Member

Choose a reason for hiding this comment

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

Why are you using a library like termcolor for terminal string styling? You can use the python built-in ANSI escape codes which are much more customizable, such as italics or underlined

setup.py Show resolved Hide resolved
@Quantalabs Quantalabs linked an issue Apr 12, 2021 that may be closed by this pull request
This was referenced Apr 12, 2021
@quantum9Innovation quantum9Innovation added wontfix 🔨 This will not be worked on and removed high-priority 🔼 This is important labels Apr 12, 2021
@quantum9Innovation
Copy link
Member Author

Using #40 instead; keeping this PR as a backup in case anything goes wrong.

@Quantalabs
Copy link
Member

Will re-open if any bad problems turn up in #40

@Quantalabs Quantalabs closed this Apr 12, 2021
Development v2 automation moved this from In progress to Done Apr 12, 2021
@quantum9Innovation quantum9Innovation added high-priority 🔼 This is important and removed wontfix 🔨 This will not be worked on labels Apr 12, 2021
Development v2 automation moved this from Done to In progress Apr 12, 2021
Co-authored-by: QLabs <55121845+Quantalabs@users.noreply.github.com>
@quantum9Innovation quantum9Innovation merged commit 935ca5d into nightly Apr 12, 2021
Development v2 automation moved this from In progress to Done Apr 12, 2021
@lgtm-com
Copy link

lgtm-com bot commented Apr 12, 2021

This pull request introduces 3 alerts when merging 73b216c into 69e6d96 - view on LGTM.com

new alerts:

  • 2 for Except block handles 'BaseException'
  • 1 for Unused import

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists feat 🚀 New feature or request high-priority 🔼 This is important
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants