-
-
Notifications
You must be signed in to change notification settings - Fork 671
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 type conversion for List
and Tuple
and their internal types
#143
Conversation
Codecov Report
@@ Coverage Diff @@
## master #143 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 252 253 +1
Lines 5296 5340 +44
=========================================
+ Hits 5296 5340 +44
Continue to review full report at Codecov.
|
1. Tuple: ensure Enum and Path elements inside a tuple are converted to their respective types (rather than retained as strings) 2. List: ensure lists that contain non-Enum and non-Path elements are not inadvertently converted to tuples
c14ef6e
to
ea9395b
Compare
Hi @tiangolo - would love to get your thoughts on this. I just pulled in the latest commits, made the code compliant with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
version: Typer==0.4.0
I experienced this issue in lazydocs project.
https://github.com/ml-tooling/lazydocs/blob/main/src/lazydocs/_cli.py#L33
Before this patch i had this error while trying to generate documentation.
'tuple' object has no attribute 'append'
Traceback (most recent call last):
File "/Users/razy69/.pyenv/versions/3.9.6/envs/test39/lib/python3.9/site-packages/lazydocs/_cli.py", line 49, in generate
generate_docs(
File "/Users/razy69/.pyenv/versions/3.9.6/envs/test39/lib/python3.9/site-packages/pysnooper/tracer.py", line 263, in simple_wrapper
return function(*args, **kwargs)
File "/Users/razy69/.pyenv/versions/3.9.6/envs/test39/lib/python3.9/site-packages/lazydocs/generation.py", line 966, in generate_docs
ignored_modules.append(module_name)
I used pysnooper to dig the issue, and it is clearly related to Typer arguments conversion.
$ lazydocs ./app --ignored-modules "main" --ignored-modules "tests" --overview-file README.md
Source path:... /Users/razy69/.pyenv/versions/3.9.6/envs/test39/lib/python3.9/site-packages/lazydocs/generation.py
Starting var:.. ignored_modules = ('main', 'tests')
I applied this patch, and it successfully fix issue.
$ lazydocs ./app --ignored-modules "main" --ignored-modules "tests" --overview-file README.md
Source path:... /Users/razy69/.pyenv/versions/3.9.6/envs/test39/lib/python3.9/site-packages/lazydocs/generation.py
Starting var:.. ignored_modules = ['main', 'tests']
@tiangolo Any update on possibly merging this fix? The workaround is minor (just add |
We were just bit by this bug ourselves. It would be nice to see this merged! Please let me know if there's any way I can assist. |
📝 Docs preview for commit 5eea0b3 at: https://62c07b152da60965a116c12a--typertiangolo.netlify.app |
List
and Tuple
and their internal types
📝 Docs preview for commit 39c07ea at: https://62c07ed8f83fa36596f231ac--typertiangolo.netlify.app |
Awesome, thanks @hellowhistler! 🚀 🍰 And thanks everyone for the discussion here! ☕ This will be available in the next release, in some hours, Typer version |
Resolves #127 and an unticketed but related issue I found when digging through the code.
Fixes:
List
parameters are properly converted to a list. Previously this worked for lists ofPath
andEnum
but all other types of lists were converted to tuples.Tuple
parameters withPath
orEnum
elements will have those elements properly converted to their respective types. Previously this worked for all other types, but these two types were retained as strings. I removed aTODO
item about recursive conversion which sounds like what I've done, but if that was referring to something else, I can re-add it.