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

Improve BasicTemplate type hints #3562

Merged
merged 8 commits into from Jun 7, 2022
Merged

Improve BasicTemplate type hints #3562

merged 8 commits into from Jun 7, 2022

Conversation

MarcSkovMadsen
Copy link
Collaborator

Fixes issues like below

image

@MarcSkovMadsen
Copy link
Collaborator Author

I don't know how you got type hints in using | @philippjfr ? These will fail when running on Python < 3.10 I believe.

image

@codecov
Copy link

codecov bot commented May 26, 2022

Codecov Report

Merging #3562 (28262f4) into master (003dbb6) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

❗ Current head 28262f4 differs from pull request most recent head 130d933. Consider uploading reports for the commit 130d933 to get more accurate results

@@            Coverage Diff             @@
##           master    #3562      +/-   ##
==========================================
- Coverage   82.82%   82.79%   -0.04%     
==========================================
  Files         199      199              
  Lines       27518    27471      -47     
==========================================
- Hits        22792    22744      -48     
- Misses       4726     4727       +1     
Impacted Files Coverage Δ
panel/template/base.py 77.36% <100.00%> (+0.04%) ⬆️
panel/widgets/input.py 94.18% <0.00%> (-2.47%) ⬇️
panel/io/resources.py 87.05% <0.00%> (-0.06%) ⬇️
panel/auth.py 41.45% <0.00%> (ø)
panel/param.py 86.51% <0.00%> (ø)
panel/config.py 59.88% <0.00%> (ø)
panel/io/rest.py 31.63% <0.00%> (ø)
panel/io/save.py 63.63% <0.00%> (ø)
panel/__init__.py 100.00% <0.00%> (ø)
panel/__main__.py 100.00% <0.00%> (ø)
... and 104 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb3c77a...130d933. Read the comment docs.

@philippjfr
Copy link
Member

The union operator is compatible with from __future__ import annotations in older versions of python. You just can't express type aliases that way.

@MarcSkovMadsen
Copy link
Collaborator Author

MarcSkovMadsen commented May 27, 2022

Thanks. Does it mean I should refactor this one to annotate using Path | str | List[Path| str] instead of Union[Path, str, List[Union[Path, str]] and add from __future__ import annotations @philippjfr ?

@MarcSkovMadsen
Copy link
Collaborator Author

Thanks. Does it mean I should refactor this one to annotate using Path | str | List[Path| str] instead of Union[Path, str, List[Union[Path, str]] and add from __future__ import annotations @philippjfr ?

I can see from #3560 (comment) that it does. I will update.

@philippjfr
Copy link
Member

Perfect, thanks!

@philippjfr
Copy link
Member

Finished typing template/base.py in this PR.

@philippjfr philippjfr merged commit b13d755 into master Jun 7, 2022
@philippjfr philippjfr deleted the template-typing branch June 7, 2022 17:01
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.

None yet

2 participants