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 for mypy errors #1362

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

woodsp-ibm
Copy link
Member

@woodsp-ibm woodsp-ibm commented May 7, 2024

Summary

CI has been failing in nightlies with mypy for couple of weeks (since the new mypy release)

qiskit_nature/second_q/operators/vibrational_op.py: note: In member "_validate_keys" of class "VibrationalOp":
Error: qiskit_nature/second_q/operators/vibrational_op.py:252: error: Call to abstract method "_validate_keys" of "SparseLabelOp" with trivial body via super() is unsafe  [safe-super]
qiskit_nature/second_q/operators/spin_op.py: note: In member "_validate_keys" of class "SpinOp":
Error: qiskit_nature/second_q/operators/spin_op.py:257: error: Call to abstract method "_validate_keys" of "SparseLabelOp" with trivial body via super() is unsafe  [safe-super]
qiskit_nature/second_q/operators/fermionic_op.py: note: In member "_validate_keys" of class "FermionicOp":
Error: qiskit_nature/second_q/operators/fermionic_op.py:199: error: Call to abstract method "_validate_keys" of "SparseLabelOp" with trivial body via super() is unsafe  [safe-super]
qiskit_nature/second_q/operators/bosonic_op.py: note: In member "_validate_keys" of class "BosonicOp":
Error: qiskit_nature/second_q/operators/bosonic_op.py:198: error: Call to abstract method "_validate_keys" of "SparseLabelOp" with trivial body via super() is unsafe  [safe-super]
qiskit_nature/second_q/operators/majorana_op.py: note: In member "_validate_keys" of class "MajoranaOp":
Error: qiskit_nature/second_q/operators/majorana_op.py:253: error: Call to abstract method "_validate_keys" of "SparseLabelOp" with trivial body via super() is unsafe  [safe-super]
Found 5 errors in 5 files (checked 324 source files)
make: *** [Makefile:48: mypy] Error 1
Error: Process completed with exit code 2.

_validate_keys() in the parent SparseLabelOp is defined as an abstract method! I removed these calls to super from the above

Details and comments

Stable branch would have the same issue but I did not label this for backporting as it includes a file (the majorana op) that is only present on main.

@woodsp-ibm
Copy link
Member Author

woodsp-ibm commented May 7, 2024

It seems the macos-latest images no longer have miniconda, As nature installs PSI4 under conda I switched for now back to macos-12. Also the macos-latest is now an ARM64 image not X86. While it seems both PSI4 and PySCF have ARM builds for now it seems simpler, to get CI running again (assuming this workaround does the trick), is just to go with the older image.

In other words to use macos-latest-large which would be the latest X86 image we would have to install miniconda in CI. I see CI uses some env var for CONDA and I do not know if this is created by the miniconda install or something that would need to be added. My goal was to get CI running and using the older image was simpler. With things running again changing up CI more to get the latest image running would be something that could be tried.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 8992324779

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.02%) to 86.942%

Files with Coverage Reduction New Missed Lines %
qiskit_nature/second_q/drivers/psi4d/psi4driver.py 1 85.12%
Totals Coverage Status
Change from base Build 8735857760: -0.02%
Covered Lines: 9022
Relevant Lines: 10377

💛 - Coveralls

1ucian0
1ucian0 previously approved these changes Jun 6, 2024
@woodsp-ibm
Copy link
Member Author

@1ucian0 Since I did this pylint released a new version and you see in Actions here the nightly run has been failing for some time. While this could be merged, if I change the branch rules as this changed the jobs, the merge CI will fail as it will fail dues to pylint. I had thought to add the necessary pylint changes/fixes to this PR as well. Either way merging just what I did so far here is no longer, on its own, going to get CI operational again.

@1ucian0
Copy link
Contributor

1ucian0 commented Jun 7, 2024

I had thought to add the necessary pylint changes/fixes to this PR as well.

sounds good!

@1ucian0 1ucian0 dismissed their stale review June 7, 2024 08:24

will be extended

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.

3 participants