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

[fud2] Experimental support for managing python environment #2265

Merged
merged 10 commits into from
Oct 21, 2024
Merged

Conversation

sgpthomas
Copy link
Collaborator

This is an experiment and I'm curious what folks think.

The main thing this PR does is to add an env subcommand that

  1. initializes a python virtualenv
  2. installs necessary python libraries
  3. updates fud2.toml to point to the virtualenv python

The point of this is to simplify the user experience of getting fud2 up and running and eases the burden of having to manually deal with installing python packages into the right place (see #2263). However, I'm not sure if we want fud2 to be in the business of managing python dependencies. The right place to solve this might actually be in documentation.

This feels very fud2 specific and so I've put this in the fud2 frontend rather than in fud-core. To accomplish this I implemented a method for extending the cli that fud-core provides. This takes advantage of argh::DynamicSubCommand. This works, but has the downside of not updating the --help screen. I might just be doing something wrong there, who knows.

@sgpthomas sgpthomas requested a review from sampsyo August 15, 2024 18:33
@sgpthomas
Copy link
Collaborator Author

Realized I didn't need the dynamic subcommands. Normal subcommands sort of work. There is the downside though that if some user of fud-core doesn't want to extend the cli. there is this empty subcommand thats generated

this relieves the burden of implementing `argh::DynamicSubCommand` from
the user of `fud_core`. Now a user can simply forward the parsing
implementations for standard `argh::SubCommand`s to extend the fud cli.
Copy link
Contributor

This pull request has not seen activity in 14 days and is being marked as stale. If you're continuing work on this, please reply and state how to get this PR in a mergeable state or what issues it is blocked on. If the PR is not ready for review, please mark it as a draft. The PR will be closed in 7 days if there is no further activity.

@sgpthomas
Copy link
Collaborator Author

Just waiting on a review for this

@github-actions github-actions bot removed the S: Stale label Oct 18, 2024
@rachitnigam
Copy link
Contributor

Does the help screen not show the new 'env' command?

@sgpthomas
Copy link
Collaborator Author

It shows up for me:
Screenshot 2024-10-19 at 3 17 16 PM

@sgpthomas
Copy link
Collaborator Author

For more context. The implementation has diverged from the original. I'm just using normal argh::Subcommands instead of the DynamicSubCommand. This means that we get help display, and adding subcommands is simpler. At the cost of having an "empty subcommand" if some user of fud_core doesn't add a command.

@rachitnigam
Copy link
Contributor

That seems fine to me! Can you add some documentation about how this command works for docs.calyxir.org? Apart from that, LGTM!

This removes the distinction between `cli` and `cli_ext` methods to
simplify the experinece for a user of `fud_core`. Naming of structures
also makes it clear that the cli can be extended.
@sgpthomas
Copy link
Collaborator Author

Added the docs, and slightly simplified the Cli Extention API, which also had the effect of not having the empty command appear if you're not using a Cli extension. Once I get the ok, I'll click the green button.

@rachitnigam
Copy link
Contributor

LGTM!

@sgpthomas sgpthomas merged commit 1028ea2 into main Oct 21, 2024
18 checks passed
@sgpthomas sgpthomas deleted the fud2-pyenv branch October 21, 2024 23:23
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.

2 participants