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

ci: updates some linting rules #73

Merged
merged 18 commits into from
Sep 11, 2023

Conversation

mrossinek
Copy link
Member

Summary

  • mypy did not run in CI at all
  • black was not run on the docs folder

I also updated the rules in the Makefile to respect the currently active Python environment (by prepending the python -m). This was also done in Qiskit Nature a while back.

Details and comments

- mypy did not run in CI at all
- black was not run on the docs folder

I also updated the rules in the Makefile to respect the currently active
Python environment (by prepending the `python -m`). This was also done
in Qiskit Nature a while back.
More to come from Elena soon... 🙂
@mrossinek mrossinek mentioned this pull request Sep 8, 2023
12 tasks
@mrossinek mrossinek mentioned this pull request Sep 8, 2023
@mrossinek mrossinek marked this pull request as ready for review September 8, 2023 16:36
Comment on lines -98 to +99
parameter_sets: Sequence[set[Parameter]],
parameter_sets: Sequence[Sequence[Parameter]],
Copy link
Member

Choose a reason for hiding this comment

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

Seeing this again here I finally commented maybe this needs further investigation. Given its called parameter_sets and a set having unique values in it, unlike Sequence - maybe it has never really been treated as a set or needed to be one. If it were assumed so and this now has duplicates, which maybe a test never covered (CI passes). @a-matsuo any comment/thought here.

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.

Thanks for getting CI going with mypy off the ground with this.

@mergify mergify bot merged commit 0d66765 into qiskit-community:main Sep 11, 2023
15 checks passed
@mrossinek mrossinek deleted the proper-linting branch September 11, 2023 08:15
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.

3 participants