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

🐛 Fix display of default value for Enum parameters inside of a list, include docs and tests #473

Merged
merged 15 commits into from
Mar 23, 2024

Conversation

asieira
Copy link
Contributor

@asieira asieira commented Oct 28, 2022

Ensures the enum .value is used in the generated help text for default Argument and Option values. Supports both isolated enum instances and lists or tuples of enum instances.

Did local testing with existing scripts/pytest.sh and all passed.

Fixes #290

asieira and others added 2 commits October 28, 2022 10:01
Ensures the enum `.value` is used in the generated help text for default Argument and Option values. Supports both isolated enum instances and lists or tuples of enum instances.

Fixes fastapi#290
@github-actions
Copy link

📝 Docs preview for commit 7f8eec1 at: https://635bd55aeef193174b41afb3--typertiangolo.netlify.app

@codecov
Copy link

codecov bot commented Oct 28, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (1e43c6b) to head (7f8eec1).
Report is 146 commits behind head on master.

❗ Current head 7f8eec1 differs from pull request most recent head 9817520. Consider uploading reports for the commit 9817520 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           master      #473      +/-   ##
===========================================
+ Coverage   96.24%   100.00%   +3.75%     
===========================================
  Files         280       280              
  Lines        5942      5945       +3     
===========================================
+ Hits         5719      5945     +226     
+ Misses        223         0     -223     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions
Copy link

📝 Docs preview for commit 711cda5 at: https://639ce9f6d8175d0b722d6157--typertiangolo.netlify.app

@jacksonwb
Copy link

Any movement here? Also experiencing this.

@svlandeg svlandeg added bug Something isn't working p2 and removed investigate labels Mar 8, 2024
@svlandeg
Copy link
Member

svlandeg commented Mar 8, 2024

Sorry for the delay on this! We're going through the PR backlog and hope to look into this one soonish. Thanks for the contribution and your patience!

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.

Thanks again for the contribution!

I ran some local tests and this looks good to me. The PR needs some tests though - I'll go ahead and add those by pushing to your branch directly, hope that's OK!

@svlandeg svlandeg marked this pull request as draft March 14, 2024 11:40
Copy link

📝 Docs preview for commit d88bd0d at: https://8cb9f299.typertiangolo.pages.dev

Copy link

📝 Docs preview for commit bc9d853 at: https://cc392f29.typertiangolo.pages.dev

Copy link

📝 Docs preview for commit f4d781d at: https://d69ffac4.typertiangolo.pages.dev

Copy link

📝 Docs preview for commit 8554833 at: https://630964bb.typertiangolo.pages.dev

@svlandeg
Copy link
Member

Ok - I'll leave this up for further review by Tiangolo.

Note that this PR now includes 2 main changes:

  • Fix the display bug - showing the enum string value instead of the Enum.name format. This is probably the original intention as the documentation already showed it like this.
  • Add a docs section and test for a parameter allowing to take a list of Enum's. I added this to be able to unit test also the display bug for a list of enums, as the original PR also covered this case.

@svlandeg svlandeg marked this pull request as ready for review March 14, 2024 13:49
@tiangolo tiangolo changed the title Fix display of default value for Enum parameters 🐛 Fix display of default value for Enum parameters inside of a list, include docs and tests Mar 23, 2024
@tiangolo
Copy link
Member

Awesome, thanks @asieira! 🚀

And thanks a lot @svlandeg for the help! 🙇

This will be available in the next hours in Typer 0.9.2 🚀

Copy link

📝 Docs preview for commit 9817520 at: https://f9bca2b0.typertiangolo.pages.dev

@tiangolo tiangolo enabled auto-merge (squash) March 23, 2024 16:30
@tiangolo tiangolo merged commit 7202e86 into fastapi:master Mar 23, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] formatting issue with enum default values in help text
4 participants