-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Fixed issue 6612 by covering ibmq examples by lint check #6649
Conversation
Hi, as some things are new to me, I'd like to discuss some points about this PR:
Thanks :) |
Hi @dhruvbhq ! Sorry we missed this one. Is your first point ("Added |
Hi @1ucian0! I haven't looked at this in a while, but I think what I did/meant was this: In the Makefile, the lint target runs pylint ... on examples/python/*.py . See here. However, the IBMQ examples are present as |
Hmm... I think pylint is recursive in all the subdirectories by default. But I did not check it. |
Could you give this another look @dhruvbhq? Would something like this make it without the
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Getting the linter running on all the scripts would be nice - they don't change much, but they should reflect decent Python practices at least.
Making things packages in Python has some unintended consequences, and I'm not sure it's really the right way to go here. The examples
scripts are all logically separate scripts, and probably should be linted as such - if pylint
won't recurse into a non-package directory on its own, it's better to do it in the shell than making a dummy package. What Luciano wrote about just specifying the directories manually is the right way here - we very rarely add new scripts so it's not a big deal to need to update a couple more lines, and this avoids the unintended consequences of creating packages.
# This code is part of Qiskit. | ||
# | ||
# (C) Copyright IBM 2017. | ||
# | ||
# This code is licensed under the Apache License, Version 2.0. You may | ||
# obtain a copy of this license in the LICENSE.txt file in the root directory | ||
# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. | ||
# | ||
# Any modifications or derivative works of this code must retain this | ||
# copyright notice, and modified files need to carry a notice indicating | ||
# that they have been altered from the originals. | ||
|
||
"""Simple examples to demonstrate the usage of Qiskit Terra.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The examples directory isn't a Python package - it's just a collection of scripts - so it shouldn't be made into a package. Doing things like this has surprising effects (mostly this is Python's fault), because editable (developer) installs of Terra will now end up making examples
an importable package whenever you open a Python interpreter. That means you wouldn't be able to access a package with that actual name any more (for example https://pypi.org/project/examples/).
(There are ways in our repository structure we can mitigate this, but really the issue is that this shouldn't be a package.)
# This code is part of Qiskit. | ||
# | ||
# (C) Copyright IBM 2017. | ||
# | ||
# This code is licensed under the Apache License, Version 2.0. You may | ||
# obtain a copy of this license in the LICENSE.txt file in the root directory | ||
# of this source tree or at http://www.apache.org/licenses/LICENSE-2.0. | ||
# | ||
# Any modifications or derivative works of this code must retain this | ||
# copyright notice, and modified files need to carry a notice indicating | ||
# that they have been altered from the originals. | ||
|
||
"""Simple examples to demonstrate the usage of Qiskit Terra on the remote IBMQ | ||
provider.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to above, this shouldn't be in a package, because the examples aren't modules, they're scripts.
pylint -rn --disable='C0103, C0114, W0621' examples/python/*.py | ||
pylint -rn --disable='C0103, W0621, E0611, E0401' examples |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With more things being added here, it would be nice to switch to using the verbose names - I can't tell at a glance what's being disabled here.
Hi, |
Hi there - sorry to come back like this, but with #8080 (and general Qiskit direction), we're going to be removing the IBMQ examples from the Terra repository because Thanks for the interest in contributing, though, and sorry it came down like this. If you're interested in finding something else, please have a look at our issues tagged "good first issue" or "help wanted". |
Summary
Fixes #6612
Details and comments
The ibmq examples were not being covered by lint checks. This PR fixes lint errors in ibmq examples and covers them by the lint checks.
__init__.py
files to convert qiskit examples to package to facilitate lint checking