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

Minor fixes #558

Merged
merged 5 commits into from
Sep 15, 2023
Merged

Minor fixes #558

merged 5 commits into from
Sep 15, 2023

Conversation

t-imamichi
Copy link
Collaborator

@t-imamichi t-imamichi commented Sep 13, 2023

Summary

Fix mypy error in main branch.
https://github.com/qiskit-community/qiskit-optimization/actions/runs/6172901069

Also fixes following items

  • qiskit.utils -> qiskit_algorithms.utils
  • minimum scipy version that supports ScipyMilpOptimizer.

Details and comments

@woodsp-ibm
Copy link
Member

I imagine this is ready since things now pass - you still have WIP in the title.

@t-imamichi
Copy link
Collaborator Author

Today's schedule CI fails due to the error.
https://github.com/qiskit-community/qiskit-optimization/actions/runs/6179936412

@t-imamichi t-imamichi changed the title (WIP) Fix mypy Fix mypy Sep 14, 2023
@t-imamichi
Copy link
Collaborator Author

I also fixed from qiskit.utils import algorithm_globals used in a qrao test.

@woodsp-ibm
Copy link
Member

Can you fix one other place too - this is a validation function that was used by algos. It will be deprecated/removed from Qiskit and was "moved" to qiskit_algorithms too.

from qiskit.utils.validation import validate_min

(Simply change from qiskit.utils... to from qiskit_algorithms.utils...

@t-imamichi t-imamichi changed the title Fix mypy Minor fixes Sep 14, 2023
@t-imamichi
Copy link
Collaborator Author

Thanks. I did it.
I also updated the minimum version of scipy to support ScipiMilpOptimizer.

@woodsp-ibm
Copy link
Member

woodsp-ibm commented Sep 14, 2023

For this

I also updated the minimum version of scipy to support ScipyMilpOptimizer.

the SciPyMilpOptimizer before, and in fact as it still is, is treated as an optional and has conditional logic around that. With bumping the version of scipy that conditional optional logic is not really needed anymore right? Maybe that optional aspect was done more due to the fact that 3.7 was still supported here but SciPy needed 3.8 or above for that version. So bottom line should we get rid of the optional logic there now with this being bumped - its irrelevant after this change.

@t-imamichi
Copy link
Collaborator Author

Thanks. I forgot about optional. I removed it.

Copy link
Member

@woodsp-ibm woodsp-ibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@t-imamichi
Copy link
Collaborator Author

t-imamichi commented Sep 15, 2023

Thanks. Could you approve it? I then merge it.

Copy link
Member

@woodsp-ibm woodsp-ibm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry my bad - the LGTM comment was supposed to have been an Approve!

@mergify mergify bot merged commit a64a824 into qiskit-community:main Sep 15, 2023
15 checks passed
@t-imamichi t-imamichi deleted the mypy branch September 15, 2023 15:01
@t-imamichi
Copy link
Collaborator Author

Thank you for merging it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants