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

✨ Implement list parsing from string with separators. #800

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

libklein
Copy link

@libklein libklein commented Apr 17, 2024

Implements feature #554.

This merge request introduces the functionality to parse List[T] options from strings using a specified separator. An example of how this can be utilized is demonstrated below:

def main(number: List[float] = typer.Option([], multiple_separator=",")):
    print(f"The sum is {sum(number)}")

#  Passing --number 1,2,3 outputs 6  
#  Works with --number 1,2 --number 3 as well. Output would be 6 again.

The implementation works by overwriting the process_value function of the click parser, doing string splitting before forwarding the result to click. This ensures full compatibility with the existing features of both Click and Typer. The feature does not allow whitespace only separators to avoid ambiguities.

Feedback is much appreciated!

Open questions:

  • Currently, error handling is implemented in the constructor of typer.Option, which delays error detection until the command execution. Should we consider moving this error handling to the OptionInfo constructor to allow for earlier failure detection?
    ToDo:
  • Add to doc_src
  • Write tests
  • Update documentation

Copy link

📝 Docs preview for commit 7eb3fd2 at: https://d9b28bcb.typertiangolo.pages.dev

Copy link

📝 Docs preview for commit cec52c9 at: https://0ff1aa3c.typertiangolo.pages.dev

@svlandeg
Copy link
Member

Hi, thanks for your PR! I'll put this in draft as long as the test suite is failing.

@svlandeg svlandeg marked this pull request as draft April 18, 2024 08:02
@svlandeg svlandeg added feature New feature, enhancement or request p3 labels Apr 18, 2024
Copy link

📝 Docs preview for commit aca33a5 at: https://d79559eb.typertiangolo.pages.dev

Copy link

📝 Docs preview for commit 46b4690 at: https://3c495073.typertiangolo.pages.dev

Copy link

📝 Docs preview for commit c0aba28 at: https://10533715.typertiangolo.pages.dev

Copy link

📝 Docs preview for commit 6233289 at: https://722bbed7.typertiangolo.pages.dev

Copy link

📝 Docs preview for commit f09da73 at: https://39b00efc.typertiangolo.pages.dev

Copy link

📝 Docs preview for commit 3cdd682 at: https://809b191b.typertiangolo.pages.dev

@libklein libklein marked this pull request as ready for review April 18, 2024 20:11
typer/core.py Outdated Show resolved Hide resolved
Copy link

📝 Docs preview for commit 03558cb at: https://9744c225.typertiangolo.pages.dev

Copy link
Contributor

@ivantodorovich ivantodorovich left a comment

Choose a reason for hiding this comment

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

Thanks 💚

docs/tutorial/multiple-values/multiple-options.md Outdated Show resolved Hide resolved
Copy link

📝 Docs preview for commit e54193b at: https://4d37e64b.typertiangolo.pages.dev

Copy link

📝 Docs preview for commit 1793f0f at: https://919ea834.typertiangolo.pages.dev

Copy link

📝 Docs preview for commit ae11002 at: https://411417d0.typertiangolo.pages.dev

Copy link
Member

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

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

Hi, thanks for the PR! It's really well written, with the appropriate changes and additions to the documentation, example code and unit tests. Really appreciated 🙏

I pushed a few smaller edits to update the documentation as well as pointing out that it does in fact work without quotes, as long as there is no whitespace inbetween, e.g.

python main.py --number 2,3,4.5

The PR looks good to merge to me, but I defer to Tiangolo for the final decision.

Copy link

github-actions bot commented Aug 9, 2024

@libklein
Copy link
Author

libklein commented Aug 9, 2024

Is there anything I can contribute?

@svlandeg
Copy link
Member

svlandeg commented Aug 12, 2024

At this point the PR is in a good shape to be reviewed by Tiangolo - we've got this on our internal board for him to review when he has time 😉 Thanks for your patience!

@svlandeg svlandeg changed the title Implement list parsing from string with separators. ✨ Implement list parsing from string with separators. Aug 26, 2024
Copy link

@libklein libklein force-pushed the multiple-arguments-via-separated-lists branch from 92a6d81 to d5328fa Compare November 30, 2024 18:16
Copy link

@dylankiss
Copy link

This would be great! Much more convenient to pass a large number of elements to an option 💯
Nice job, @libklein. Hope it will be merged soon 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature, enhancement or request p3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOC: Documentation of passing multiple values in "option" vs "argument" isn't sufficiently explicit
4 participants