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

feat: Allow to pass entry point to clipstick parse function #57

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

playpauseandstop
Copy link

This is helpful, when list of CLI args comes not from sys.argv and, as result, entry point cannot be parsed from sys.argv[0].

This is also might be helpful for cases, when some CLI called via python3 -m ... and sys.argv[0] as result contain meaningless value of something/__main__.py (however, looks like, the proper fix for that case is out of scope of current change).

ps. Thanks a lot for clipstick library 🙏 Very appreciate your work, as it makes my life much more easier 🙇

This is helpful, when list of CLI args comes not from `sys.argv` and,
as result, entry point cannot be parsed from `sys.argv[0]`.

This is also might be helpful for cases, when some CLI called via
`python3 -m ...` and `sys.argv[0]` as result contain meaningless value
of `something/__main__.py` (however, looks like, the proper fix for that
case is out of scope of current change).
@playpauseandstop
Copy link
Author

@sander76

Adding some context to my PR.

I needed to provide a CLI utilities which at same time will be called from CLI as python3 path/to/cli.py ... and within other Python code as,

from path.to import cli

cli.main(...)

To achieving that I'm doing little dances with main function,

def main(*argv: str) -> int:
    args = clipstick.parse(CliArgs, argv or sys.argv[1:])
    ...

and all works perfectly fine, but little perfectionist in myself want to see real tool name in help output instead of my-cli-app, which comes from DUMMY_ENTRY_POINT constant.

Cause of that I've come up with such PR.

Please let me know, if it seems reasonable for you or you have other ideas here.

def parse(
model: type[TPydanticModel],
args: list[str] | None = None,
entry_point: str | None = None,
Copy link
Owner

Choose a reason for hiding this comment

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

Hi @playpauseandstop thanks for the pr! Much appreciated!
My use of the DUMMY_ENTRYPOINT has always bugged me a bit.
Your pr makes me think about this a bit more.

Would it be an alternative that the args: list[str] should always have the entrypoint as a first item, just like sys.argv?
This removes the entry_point parameter, the DUMMY_ENTRY_POINT and is consistent with sys.argv.

I am not sure I completely understand your usecase as you describe below. But I guess you can then use the above solution too ?

def main(*argv: str) -> int:
    args = clipstick.parse(CliArgs, argv or sys.argv[1:])
...

Copy link
Owner

Choose a reason for hiding this comment

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

Forget about my above comment. I have been reading up on this on how other libs do this. for example:

https://docs.python.org/3/library/argparse.html#prog

I guess your suggestions is the proper approach.

@sander76
Copy link
Owner

Hi @playpauseandstop. I think have not documented this properly, but can you point your pr to my dev branch ?

@sander76
Copy link
Owner

also thank for your remark about __main__.py. I've created an issue for this: #58

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