-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Implement and enforce explicit re-export for cirq modules #6722
Conversation
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.
Please, also update the cirq-core/cirq/__init__.py
and remove any wip code such as from foo import bar
.
Make sure the CI-reported errors are all addressed. You may want to setup a local development environment following https://github.com/quantumlib/Cirq/blob/main/docs/dev/development.md#setting-up-an-environment.
After that you can use check/mypy
, check/pylint
to run the respective CI tests locally.
Running check/format-incremental --apply
would fix code formatting of any changed files.
Thank you for taking this on!
@@ -12,6 +12,7 @@ | |||
# See the License for the specific language governing permissions and | |||
# limitations under the License. | |||
"""Experiments and tools for characterizing quantum operations.""" | |||
from foo import bar as bar |
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.
Please remove as well as other debugging code.
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.
Sure I will work on that
I have made a push now, please review the pull request @pavoljuhas |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6722 +/- ##
==========================================
- Coverage 97.83% 97.83% -0.01%
==========================================
Files 1077 1077
Lines 92487 92485 -2
==========================================
- Hits 90485 90483 -2
Misses 2002 2002 ☔ View full report in Codecov by Sentry. |
No need for re-export from `cirq._import`.
…cuit Just import it from cirq.
Import it from the actual defining module.
Just import from cirq. The caller is in the cirq_ionq subpackage.
Executed check/format-incremental --apply
These are used for package initialization and are not for export.
These are already added to super module namespace on import.
Disable for very long re-export statements.
@ashiq-firoz - I have updated your PR so that the If this passes the CI, I feel it should be good to go. |
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.
LGTM.
…#6722) * Enable and enforce the `no_implicit_reexport` mypy rule for cirq modules * Update `__init__.py` files so they explicitly re-export public symbols, but do not re-export local symbols or submodules already in parent namespace * Fix few instances of imports from incorrect modules Fixes quantumlib#6717 --------- Co-authored-by: Pavol Juhas <juhas@google.com>
I have updated mypy.ini inside dev_tools and also made modifications in all init.py files to match
Ref: #6717