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

Type hints in 0.13.0 incompatible with Python 3.7 #893

Closed
mtreinish opened this issue Jun 7, 2023 · 8 comments
Closed

Type hints in 0.13.0 incompatible with Python 3.7 #893

mtreinish opened this issue Jun 7, 2023 · 8 comments
Labels
bug Something isn't working
Milestone

Comments

@mtreinish
Copy link
Member

Information

  • rustworkx version: 0.13.0
  • Python version: 3.7
  • Rust version: N/A
  • Operating system: Linux

What is the current behavior?

Running mypy with Python 3.7 in a package that uses rustworkx fails trying to parse the stubs files. For example:

/opt/hostedtoolcache/Python/3.7.16/x64/lib/python3.7/site-packages/rustworkx/graph.pyi:34: error: invalid syntax  [syntax]

See: https://github.com/qiskit-community/qiskit-nature-pyscf/actions/runs/5200238443/jobs/9378913497?pr=35#step:7:15 for where this happened in CI.

What is the expected behavior?

The type hint syntax used is compatible with the rustworkx python version

Steps to reproduce the problem

Run mypy using rustworkx on python 3.7

@IvanIsCoding
Copy link
Collaborator

I will try to fix it next week, but it looks it is a problem with __init__

@IvanIsCoding
Copy link
Collaborator

IvanIsCoding commented Jun 24, 2023

It turns out that the error is due to PEP 570. So this would be a big rewrite which actually hurts Python 3.8+ users

@mtreinish
Copy link
Member Author

Hmm, then I think considering Python 3.7 is going eol; for 0.13.1 let's just write a known issue release note about the lack of type hint compatibility with python 3.7

@IvanIsCoding
Copy link
Collaborator

Hmm, then I think considering Python 3.7 is going eol; for 0.13.1 let's just write a known issue release note about the lack of type hint compatibility with python 3.7

If we could just purge the .pyi files from the 3.7 release and keep it in the other ones that would be the best scenario

@IvanIsCoding
Copy link
Collaborator

I think we should backport #907 to the main branch, release 0.13.1 for all versions except 3.7. And then have another branch for 3.7 but with a modified MANIFEST.in exluding https://github.com/Qiskit/rustworkx/blob/main/MANIFEST.in#L7 which remove the stub files from Python 3.7

@mtreinish
Copy link
Member Author

I don't think it's really worth the trouble. Given that Python 3.7 is already EoL (since 06/27/2023) I think it's probably sufficient to just document the type hints are incompatible with Python 3.7 as it doesn't effect the actual execution of the library on Python 3.7 it's just for using mypy to validate against the stub files. I'm preparing the release notes for the 0.13.1 release right now. I'll include this in that PR.

mtreinish added a commit to mtreinish/retworkx that referenced this issue Jul 25, 2023
This commit prepares the 0.13.1 release, this consists of bumping the
version number to 0.13.1 in all the appropriate files. It also adds a
prelude release note to provide a brief introduction to the release. When
this commit merges it should be tagged as the 0.13.1 release.

Closes Qiskit#893
@mtreinish mtreinish mentioned this issue Jul 25, 2023
2 tasks
@IvanIsCoding
Copy link
Collaborator

I don't think it's really worth the trouble. Given that Python 3.7 is already EoL (since 06/27/2023) I think it's probably sufficient to just document the type hints are incompatible with Python 3.7 as it doesn't effect the actual execution of the library on Python 3.7 it's just for using mypy to validate against the stub files. I'm preparing the release notes for the 0.13.1 release right now. I'll include this in that PR.

I think that is fair. We can also add to the note that mypy can type check the code at a higher Python version in CI

mtreinish added a commit to mtreinish/retworkx that referenced this issue Jul 27, 2023
This commit prepares the 0.13.1 release, this consists of bumping the
version number to 0.13.1 in all the appropriate files. It also adds a
prelude release note to provide a brief introduction to the release. When
this commit merges it should be tagged as the 0.13.1 release.

Closes Qiskit#893
enavarro51 added a commit that referenced this issue Jul 29, 2023
* Prepare 0.13.1 release

This commit prepares the 0.13.1 release, this consists of bumping the
version number to 0.13.1 in all the appropriate files. It also adds a
prelude release note to provide a brief introduction to the release. When
this commit merges it should be tagged as the 0.13.1 release.

Closes #893

* Add missing fix notes

* Apply suggestions from code review

Co-authored-by: Edwin Navarro <[email protected]>

---------

Co-authored-by: Edwin Navarro <[email protected]>
@IvanIsCoding
Copy link
Collaborator

Closing after the release of 0.13.1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants