typer icon indicating copy to clipboard operation
typer copied to clipboard

🐛 Fix support for UnionType with Python 3.11

Open jonaslb opened this issue 2 years ago • 10 comments

Fixes #533, which already had good details and a proposal for a solution, based on using get_args and get_origin from the typing module instead of the __args__ and __origin__ attributes. So I went and did that.

We also needed to actually handle the UnionType in addition to the Union from before.

Now there was a bit of a headache in terms of writing this in a backwards compatible way. What I've done is place the compatibility code into _compat_utils.py. But let me know if you'd prefer to keep it in main.py.

I also added a single test, just an adapted copy of another one with the x | None syntax. It is conditioned on Python >= 3.10, so it shouldn't fail on older Pythons.

jonaslb avatar Feb 04 '23 02:02 jonaslb

📝 Docs preview for commit 1f9dd82290ca60804f8d4af3e7b0e318d090ecdc at: https://63ddbef8955c2d354bae0bbd--typertiangolo.netlify.app

github-actions[bot] avatar Feb 04 '23 02:02 github-actions[bot]

It looks like mypy behaves differently in the different tests across versions. 3.6, 3.10 and 3.11 are ok, but 3.7, 3.8 and 3.9 are not. That seems confusing. I can maybe look at it another day again, otherwise if someone can see why it happens let me know.

jonaslb avatar Feb 04 '23 02:02 jonaslb

📝 Docs preview for commit 84ad04cea7d663f7faac7561582cc3b65d02e9eb at: https://63e1682403f85605237e7c38--typertiangolo.netlify.app

github-actions[bot] avatar Feb 06 '23 20:02 github-actions[bot]

Failures are down to mypy version I think, it works fine here with latest. I see some errors in other places though (unrelated to my changes).

Also realized there is already another PR for this issue (just references other issue numbers, so I hadn't seen it): #522

Regardless, hope to see either PR merged. Ping @tiangolo

jonaslb avatar Feb 06 '23 20:02 jonaslb

@tiangolo - can we merge this please?

austin-silk avatar Oct 02 '23 15:10 austin-silk

Rebased on master and now all is green 👍

Still happy to make adaptations or answer questions.

jonaslb avatar Oct 03 '23 15:10 jonaslb

@svlandeg I gather that #676 is the preferred solution to the UnionType situation? Should I just close this? Will we ever get anything on the topic merged - ie. what's blocking?

jonaslb avatar Mar 29 '24 20:03 jonaslb

Hi @jonaslb, we've been going through the backlog of PRs to label them and connect related ones. We've already reviewed, adapted and merged a bunch for last week's releases 0.9.1 through 0.11.0. Open-source maintenance costs time and effort as I hope you can appreciate ;-) Thanks for your contributions (and patience) so far, we'll definitely get to this! It's fine to keep this one open in the meantime.

svlandeg avatar Mar 29 '24 23:03 svlandeg

Open-source maintenance costs time and effort as I hope you can appreciate ;-)

Of course, and it's great to see that time is now being spent on typer also from the maintainer side ;-) I'm looking forward to engaging about details here if this PR turns out to be the preferred one out of the many submitted on the topic.

jonaslb avatar Mar 29 '24 23:03 jonaslb

Ruff tries to automatically fix these Optional[str] in my parameters and I had to disable it, so I'm really looking forward to seeing this merged and released.

lachaib avatar Jun 03 '24 10:06 lachaib

@tiangolo I understand you have other time commitments and it must be overwhelming, but I would like to have this PR merged whenever possible. I use Ruff with pyupgrade rules like @lachaib and I find the lack of the modern union support a considerable hindrance when using Typer and Ruff together.

luispsantos avatar Jul 11 '24 11:07 luispsantos

Great! Thank you for your contribution @jonaslb! :cake:

And in particular, thank you for your patience and your kindness. :hugs:

Thank you @svlandeg for the great job as always investigating, reviewing, and reporting everything. :bow:

This will be available in Typer 0.12.4, released in the next hours. :tada:

tiangolo avatar Aug 17 '24 02:08 tiangolo