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

ENH: Add cli framework #44

Merged
merged 6 commits into from
Sep 19, 2024
Merged

ENH: Add cli framework #44

merged 6 commits into from
Sep 19, 2024

Conversation

tangkong
Copy link
Contributor

@tangkong tangkong commented Sep 16, 2024

Description

  • Add a cli framework (that I keep lifting from atef)
  • Fix versioning entrypoint (beams.__version__)
  • Fixes a bit of a logging oversight, don't disable existing loggers when setup_logging is called

Motivation and Context

In partial response to #10 . When this is in our environment we won't be able to ask users to find the makefile and run commands with it. This gives us standard python cli entrypoints.

I could think of a few entrypoints we might add, but didn't want to jump the gun before we discussed them.

  • beams run: run a provided tree. I added this subcommand as an example, but it could use more arguments (tick rate, tick mode {continuous, fixed number, idk}, ...)
  • beams start-server: start the sequencer server that's currently invoked via make run_sequencer
  • beams test_ioc: start one of the test iocs we have bundled. Maybe this is replaced by a caproto call.

How Has This Been Tested?

No new tests added. interactive testing predominantly

Where Has This Been Documented?

This PR.

cli is also self-documenting now:

$ beams -h
usage: beams [-h] [--version] [--log LOG_LEVEL] {run} ...

`beams` is the top-level command for accessing various subcommands.

    $ beams run --help

positional arguments:
  {run}                 Possible subcommands

optional arguments:
  -h, --help            show this help message and exit
  --version, -V         Show the beams version number and exit.
  --log LOG_LEVEL, -l LOG_LEVEL
                        Python logging level (e.g. DEBUG, INFO, WARNING)
$ beams run -h
usage: beams run [-h] filepath

`beams run` runs a behavior tree given a configuration file

positional arguments:
  filepath    Behavior Tree configuration filepath

optional arguments:
  -h, --help  show this help message and exit

Pre-merge checklist

  • Code works interactively
  • Code follows the style guide
  • Code contains descriptive docstrings, including context and API
  • New/changed functions and methods are covered in the test suite where possible
  • Test suite passes locally
  • Test suite passes on GitHub Actions
  • Ran docs/pre-release-notes.sh and created a pre-release documentation page
  • Pre-release docs include context, functional descriptions, and contributors as appropriate

@ZLLentz
Copy link
Member

ZLLentz commented Sep 16, 2024

I've never been happy with this re-used cli setup but I've never suggested a better way
I'm mostly upset about this:

$ time atef --version
1.5.0

real    0m11.011s
user    0m6.788s
sys     0m1.302s

atef has tons of overhead for doing nothing in the cli, largely in import statements

@tangkong
Copy link
Contributor Author

tangkong commented Sep 16, 2024

True, maybe this is the time to try and streamline this a bit.

ZLLentz
ZLLentz previously approved these changes Sep 16, 2024
Copy link
Member

@ZLLentz ZLLentz left a comment

Choose a reason for hiding this comment

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

I don't have any particularly constructive objections, I would have preferred defining what we want to do here prior to standing up the framework though.

@tangkong
Copy link
Contributor Author

I'm happy to defer this until we tagup and discuss what functionality we want. I just figured we couldn't be exposing a makefile when this is in our env

@ZLLentz
Copy link
Member

ZLLentz commented Sep 16, 2024

It's true, make is a developer tool and we should define what gets run in prod and by users.

@joshc-slac
Copy link
Collaborator

I like this too seems pythonic. make is definitely more principic1 to c / c++ style development things. I agree lets dream up program entrypoint stuff

Footnotes

  1. After looking far and wide and literally talking to my friend with a graduate degree in linguistics I have to conclude I made this word up at some point in my life. principic : relating to the principles or ethos of

@tangkong
Copy link
Contributor Author

I'll re-request reviews after this minor refactor, should be good enough as is for a first pass framework setup

@tangkong
Copy link
Contributor Author

I received verbal approval from josh during out sync yesterday, and will be merging this. We can refine the command selection later

@tangkong tangkong merged commit 3b8b95b into pcdshub:master Sep 19, 2024
7 of 11 checks passed
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.

3 participants