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

"Dynamic" recipes #28

Closed
nicogelders opened this issue Feb 17, 2022 · 2 comments
Closed

"Dynamic" recipes #28

nicogelders opened this issue Feb 17, 2022 · 2 comments

Comments

@nicogelders
Copy link

nicogelders commented Feb 17, 2022

Make recipes take variables so that they could be reused more easily.

Example:

The recipe:

max_cells = RecipeInfo(
    src="len(nb.cells) < amount_max",
    description="Assert that there are less than 'amount_max' cells in the notebook.",
)

The command:

databooks assert path/to/nbs --recipe max_cells --amount_max 10
databooks assert path/to/nbs --recipe max_cells --help

Would return the arguments of that recipe

@nicogelders
Copy link
Author

The --help thing might be tricky maybe do something like this

databooks inspect-recipe max_cells

@murilo-cunha
Copy link
Member

After some thought, I see 3 ways of accomplishing this:

  1. We could allow extra arguments and if they are passed we replace the string with the right values
  2. We could dynamically create subcommands - one for each recipe with it's own help and call each subcommand after assert has run
  3. Create another command entirely for recipes

Some points to consider:

  • What if the user passes multiple recipes?
  • What if the arguments have similar names?
  • How can we enforce that the arguments go to the correct recipe?

As of now, I do not see a feature like this because

  • It may make the app less intuitive for users and add some unnecessary complexity
  • We only have one recipe that it'd make sense to pass arguments (max-cells)
  • If a user really wants another value, he/she can easily do this by passing the expression instead of the recipe + argument (databooks assert "len(nb.cells) < 10")

That being said, I'm open to revisit this once we have more recipes/there's community interest in a feature like this. If there's a simple way to go around this I'm also open to review any PRs regarding this.

Thanks for your suggestion! 🚀

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

No branches or pull requests

2 participants