Skip to content

CI - run pylint checks with dependencies installed #6163

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

Merged

Conversation

pavoljuhas
Copy link
Collaborator

dill-0.3.6 is out and hopefully will not regress #5383

dill-0.3.6 is out and hopefully will not regress quantumlib#5383
Allow pylint to check arguments in function calls.
Use a single shape argument in numpy array.reshape().
@pavoljuhas pavoljuhas force-pushed the ci-check-pylint-with-cirq-deps branch from c9d2bde to 81a14e7 Compare June 24, 2023 01:30
@pavoljuhas pavoljuhas marked this pull request as ready for review June 24, 2023 01:42
@pavoljuhas pavoljuhas requested review from a team, vtomole and cduck as code owners June 24, 2023 01:42
@pavoljuhas pavoljuhas requested a review from dstrain115 June 24, 2023 01:42
Copy link
Collaborator

@viathor viathor left a comment

Choose a reason for hiding this comment

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

FWIW, the method on ndarray does allow one to pass dimensions as separate arguments:

Unlike the free function numpy.reshape, this method on ndarray allows the elements of the shape parameter to be passed in as separate arguments. For example, a.reshape(10, 11) is equivalent to a.reshape((10, 11)).

according to documentation.

Still, I suppose it is a little bit more future-proof to write it this way in case it gets refactored.

@pavoljuhas
Copy link
Collaborator Author

Still, I suppose it is a little bit more future-proof to write it this way in case it gets refactored.

Yes, the reshape signature with overloads is confusing enough for pylint to complain.
The main benefit of this PR is to make check/pylint pass in a local development environment.
The only reason it worked in the CI was that numpy was not installed and so pylint could not see reshape() arguments.

Thanks for the review!

@pavoljuhas pavoljuhas enabled auto-merge (squash) June 27, 2023 18:50
@pavoljuhas pavoljuhas merged commit 6565fc5 into quantumlib:master Jun 27, 2023
@pavoljuhas pavoljuhas deleted the ci-check-pylint-with-cirq-deps branch June 27, 2023 20:42
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
* Install all cirq dependencies for pylint check so 
  pylint can verify arguments in function calls.
* Fix pylint complaints on too-many-function-args by
  using a single shape argument for numpy array.reshape().
* Remove obsolete pin of the dill version.
  The current release dill-0.3.6 seems OK w/r to quantumlib#5383
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.

2 participants