qiskit-community / qiskit-nature

Qiskit Nature is an open-source, quantum computing, framework for solving quantum mechanical natural science problems.
https://qiskit-community.github.io/qiskit-nature/
Apache License 2.0
290 stars 197 forks source link

Fix for mypy errors #1362

Open woodsp-ibm opened 2 months ago

woodsp-ibm commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

Pull Request Test Coverage Report for Build 8992324779

Details


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

💛 - Coveralls
woodsp-ibm commented 1 month ago

@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 commented 1 month ago

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

sounds good!