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

Deprecating configuring through PEP #455

Open
nsheff opened this issue Feb 15, 2024 · 3 comments
Open

Deprecating configuring through PEP #455

nsheff opened this issue Feb 15, 2024 · 3 comments
Milestone

Comments

@nsheff
Copy link
Contributor

nsheff commented Feb 15, 2024

When configuring through a PEP, we get this warning:

This PEP configures looper through the project config. This approach is deprecated and will be removed in future versions. Please use a looper config file. For more information see looper.databio.org/en/latest/looper-config

Maybe it's time to do this.

Looper config short arg name

--looper-config is verbose and has no short arg name. yet, it seems to me to be a commonly used argument. It should have a short arg name.

The question is, what should it be? -c seems obvious, but that's in use for compute. Still, I'd rather use -c for --looper-config than for --compute.

It could be -l, but that's in use for --limit.

Or maybe it should take over the positional argument of the PEP. Yes, that's probably it.

This will break backwards compatibility, but it's probably time to do that. My thought is:

  • use position argument for looper config
  • use --pep to override the pep.
  • I guess this could be looper v2.0.0 since it breaks backwards compatibility in the CLI.
@nsheff nsheff changed the title Looper config short arg name Deprecating configuring through PEP Feb 15, 2024
@nsheff nsheff added this to the v2.0.0 milestone Feb 16, 2024
@donaldcampbelljr
Copy link
Contributor

pydantic-argparse does not support positional arguments:

https://pydantic-argparse.supimdos.com/background/

Opinionated Design
pydantic-argparse is a very opinionated package by design. It aims for a simple API, and to be both full featured while limiting excessive choices. For example, there are no positional arguments in pydantic-argparse; only optional and required arguments. If your opinions do not align with these design choices, then you may not want to use the package.

Could we rename:
--pep to override the pep.
-c, --config for the looper config
-g for compute?

@donaldcampbelljr
Copy link
Contributor

maybe: take out short form of compute and just use long form

@nsheff nsheff added this to PEP Jun 25, 2024
@donaldcampbelljr
Copy link
Contributor

Ok, I've refactored this to be:
--pep-config for the pep
--config, -c for the looper config
--compute for compute

I've removed config_file argument so that now looper is only aware of the looper config and the pep config

These changes are in the above PR which should only be merged to dev after 1.9.0 is released as our last backwards compatible release before 2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants