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

Pin ipywidgets #9272

Merged
merged 1 commit into from
Dec 9, 2022
Merged

Pin ipywidgets #9272

merged 1 commit into from
Dec 9, 2022

Conversation

t-imamichi
Copy link
Member

@t-imamichi t-imamichi commented Dec 9, 2022

Summary

ipywidgets 8.0.3 (released on Dec 7 link) raises a deprecation warning as follows.

======================================================================
ERROR: test_plot_error_map_over_100_qubit_backend_v2 (test.python.visualization.test_gate_map.TestGateMap)
test.python.visualization.test_gate_map.TestGateMap.test_plot_error_map_over_100_qubit_backend_v2
----------------------------------------------------------------------
testtools.testresult.real._StringException: Traceback (most recent call last):
  File "/Users/ima/tasks/2_2022/qiskit/terra/test/python/visualization/test_gate_map.py", line 151, in test_plot_error_map_over_100_qubit_backend_v2
    fig = plot_error_map(backend)
  File "/Users/ima/tasks/2_2022/qiskit/terra/qiskit/utils/lazy_tester.py", line 165, in out
    return function(*args, **kwargs)
  File "/Users/ima/tasks/2_2022/qiskit/terra/qiskit/utils/lazy_tester.py", line 164, in out
    self.require_now(feature)
  File "/Users/ima/tasks/2_2022/qiskit/terra/qiskit/utils/lazy_tester.py", line 221, in require_now
    if self:
  File "/Users/ima/tasks/2_2022/qiskit/terra/qiskit/utils/lazy_tester.py", line 114, in __bool__
    self._bool = self._is_available()
  File "/Users/ima/tasks/2_2022/qiskit/terra/qiskit/utils/lazy_tester.py", line 289, in _is_available
    imported = importlib.import_module(module)
  File "/usr/local/Cellar/python@3.9/3.9.15/Frameworks/Python.framework/Versions/3.9/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 850, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "/Users/ima/envs/dev39/lib/python3.9/site-packages/seaborn/__init__.py", line 12, in <module>
    from .widgets import *  # noqa: F401,F403
  File "/Users/ima/envs/dev39/lib/python3.9/site-packages/seaborn/widgets.py", line 6, in <module>
    from ipywidgets import interact, FloatSlider, IntSlider
  File "/Users/ima/envs/dev39/lib/python3.9/site-packages/ipywidgets/__init__.py", line 25, in <module>
    from .widgets import *
  File "/Users/ima/envs/dev39/lib/python3.9/site-packages/ipywidgets/widgets/__init__.py", line 4, in <module>
    from .widget import Widget, CallbackDispatcher, register, widget_serialization
  File "/Users/ima/envs/dev39/lib/python3.9/site-packages/ipywidgets/widgets/widget.py", line 300, in <module>
    class Widget(LoggingHasTraits):
  File "/Users/ima/envs/dev39/lib/python3.9/site-packages/traitlets/traitlets.py", line 972, in __init__
    cls.setup_class(classdict)
  File "/Users/ima/envs/dev39/lib/python3.9/site-packages/traitlets/traitlets.py", line 1009, in setup_class
    value = getattr(cls, name)
  File "/Users/ima/envs/dev39/lib/python3.9/site-packages/ipywidgets/widgets/widget.py", line 296, in __get__
    return self.fget()
  File "/Users/ima/envs/dev39/lib/python3.9/site-packages/ipywidgets/widgets/widget.py", line 324, in _active_widgets
    deprecation("Widget._active_widgets is deprecated.")
  File "/Users/ima/envs/dev39/lib/python3.9/site-packages/ipywidgets/widgets/utils.py", line 64, in deprecation
    warnings.warn(message, DeprecationWarning, stacklevel=stacklevel+1)
DeprecationWarning: Widget._active_widgets is deprecated.

Details and comments

@t-imamichi t-imamichi requested a review from a team as a code owner December 9, 2022 08:48
@qiskit-bot
Copy link
Collaborator

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

  • @Qiskit/terra-core

@coveralls
Copy link

coveralls commented Dec 9, 2022

Pull Request Test Coverage Report for Build 3656342382

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.04%) to 84.556%

Totals Coverage Status
Change from base Build 3650169352: -0.04%
Covered Lines: 63222
Relevant Lines: 74769

💛 - Coveralls

@Cryoris
Copy link
Contributor

Cryoris commented Dec 9, 2022

I'm a bit confused by this warning: I can reproduce it with running the test but not if I use the function that gives rise to the warning directly:

>>> from qiskit.providers.fake_provider import FakeParis
>>> from qiskit.visualization import plot_error_map
>>> plot_error_map(FakeParis())  # this raises the warning in the test it seems
<Figure size 2400x1800 with 5 Axes>  # no warning here!

In the ipywidgets it also seems the warning is only raised in a certain condition -- which makes me wonder if the test should be set up differently or whether it's really a issue with ipywidgets 🤔

@t-imamichi
Copy link
Member Author

t-imamichi commented Dec 9, 2022

Yes, the condition looks complicated. This PR jupyter-widgets/ipywidgets#3569 introduced it. But I haven't figured out it yet.

@mtreinish
Copy link
Member

Looking at that code it is definitely an issue with ipywidgets (I'm inclined to think it's a bug in their stack inspection code added in that PR) and/or seaborn. The test itself isn't doing anything suspicious in that all that is happening is that seaborn is getting imported as part of the visualization code and triggering a deprecation warning which we treat as fatal in the test suite (so we update our code when an upstream project deprecates an api. If we don't want to pin ipywidgets here the other option is to add the deprecation warnings to the exclude list: https://github.com/Qiskit/qiskit-terra/blob/main/qiskit/test/base.py#L190-L226 which I think is also valid in this case

Comment on lines +10 to +12
# ipywidgets 8.0.3 started emitting deprecation warnings via a seaborn import.
# This pin can be removed when compatibility with those packages is fixed.
ipywidgets<8.0.3
Copy link
Member

Choose a reason for hiding this comment

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

Was there a case just pinning in the requirements list wasn't enough and we needed to pin it in the constraints file too?

Copy link
Member Author

Choose a reason for hiding this comment

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

This line upgrades ipywidgets to 8.0.3 without this change of constraints.txt.
https://github.com/Qiskit/qiskit-terra/blob/ee4a5712bfac6104e3e93fc8a3ff845a3a0ddd77/.azure/test-linux.yml#L156

Copy link
Member

Choose a reason for hiding this comment

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

We probably don't need the requirements-dev.txt change in parallel then. But it doesn't hurt anything either.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, you are right... I didn't have to change in parallel.

@t-imamichi
Copy link
Member Author

Thank you for looking at the code, Matthew. I didn't know about allow_DeprecationWarning_modules. You can choose the best way to fix this issue.

Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

I'm fine with pinning it's just for testing so that works as well as anything else as long as we remember to circle back after the warnings are fixed

@mtreinish mtreinish added automerge Changelog: None Do not include in changelog type: qa Issues and PRs that relate to testing and code quality labels Dec 9, 2022
@mergify mergify bot merged commit b4f4c34 into Qiskit:main Dec 9, 2022
@t-imamichi t-imamichi deleted the pin-ipywidgets branch December 9, 2022 15:48
@t-imamichi t-imamichi added the stable backport potential The bug might be minimal and/or import enough to be port to stable label Dec 12, 2022
mergify bot pushed a commit that referenced this pull request Dec 12, 2022
(cherry picked from commit b4f4c34)
Cryoris pushed a commit that referenced this pull request Dec 21, 2022
* fix tox.ini (#9276)

(cherry picked from commit e70ce50)

# Conflicts:
#	tox.ini

* Fix merge conflict

* pin ipywidgets (#9272)

(cherry picked from commit b4f4c34)

Co-authored-by: Takashi Imamichi <31178928+t-imamichi@users.noreply.github.com>
Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
Co-authored-by: Takashi Imamichi <imamichi@jp.ibm.com>
Cryoris pushed a commit to Cryoris/qiskit-terra that referenced this pull request Jan 12, 2023
jakelishman added a commit to jakelishman/qiskit-terra that referenced this pull request Jan 12, 2023
These were originally added in Qiskit#9105 and Qiskit#9272 respectively, but the
original problem package `seaborn` has released since then, which may
have fixed things.
jakelishman added a commit to jakelishman/qiskit-terra that referenced this pull request Jan 12, 2023
These were originally added in Qiskit#9105 and Qiskit#9272 respectively, but the
original problem package `seaborn` has released since then, which may
have fixed things.

This removes some now-unnecessary suppressions from image-related
packages, and adds the new suppression for the pyzmq problem, which is
Jupyter's domain to handle.  The extra environment variable in the
images test run is to eagerly move to new default behaviour starting in
jupyter-core 6; there is no need for us to pin the package too low,
since this warning is just encouraging people to proactively test the
new behaviour, and it doesn't cause our suite problems.
jakelishman added a commit to jakelishman/qiskit-terra that referenced this pull request Jan 12, 2023
These were originally added in Qiskit#9105 and Qiskit#9272 respectively, but the
original problem package `seaborn` has released since then, which may
have fixed things.

This removes some now-unnecessary suppressions from image-related
packages, and adds the new suppression for the pyzmq problem, which is
Jupyter's domain to handle.  The extra environment variable in the
images test run is to eagerly move to new default behaviour starting in
jupyter-core 6; there is no need for us to pin the package too low,
since this warning is just encouraging people to proactively test the
new behaviour, and it doesn't cause our suite problems.
jakelishman added a commit to jakelishman/qiskit-terra that referenced this pull request Jan 12, 2023
These were originally added in Qiskit#9105 and Qiskit#9272 respectively, but the
original problem package `seaborn` has released since then, which may
have fixed things.

This removes some now-unnecessary suppressions from image-related
packages, and adds the new suppression for the pyzmq problem, which is
Jupyter's domain to handle.  The extra environment variable in the
images test run is to eagerly move to new default behaviour starting in
jupyter-core 6; there is no need for us to pin the package too low,
since this warning is just encouraging people to proactively test the
new behaviour, and it doesn't cause our suite problems.
mergify bot pushed a commit that referenced this pull request Jan 12, 2023
* Relax constraints on jupyter-core and ipywidgets

These were originally added in #9105 and #9272 respectively, but the
original problem package `seaborn` has released since then, which may
have fixed things.

* Fix suppressions for Jupyter warnings

This removes some now-unnecessary suppressions from image-related
packages, and adds the new suppression for the pyzmq problem, which is
Jupyter's domain to handle.  The extra environment variable in the
images test run is to eagerly move to new default behaviour starting in
jupyter-core 6; there is no need for us to pin the package too low,
since this warning is just encouraging people to proactively test the
new behaviour, and it doesn't cause our suite problems.

* Use correct YAML syntax

One day I'll remember this when writing environment variables in YAML
files, but it's not this day.
mergify bot pushed a commit that referenced this pull request Jan 12, 2023
* Fix NumPy 1.24.0 compatibility and pin `coverage<7.0` (#9305)

* fix Kraus from (array, None)

* fix triu_to_dense test

* fix instruction comparison

* skip snobfit if numpy 1.24.0 or above is installed

Co-authored-by: ElePT <57907331+ElePT@users.noreply.github.com>

* pin coverage <7.0

* add links to Kraus and snobfit issues

* retrigger CI

Co-authored-by: ElePT <57907331+ElePT@users.noreply.github.com>
(cherry picked from commit 9733fc0)

* Update `qiskit.utils.wrap_method` for Python 3.11 (#9310)

* Revert "[Test] Pin maximum python version in CI to <3.11.1 (#9296)"

This reverts commit 07e0a2f.

* Do not treat __init_subclass__ as a special type method

* Release note

* Apply suggestions from code review

Co-authored-by: Julien Gacon <gaconju@gmail.com>

* Use inspect.getattr_static to bypass descriptor call

* Update release note

* Update wrap_method test

* Adjust wording on release note

Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 0344a1c)

* Relax constraints on jupyter-core and ipywidgets (#9364)

These were originally added in #9105 and #9272 respectively, but the
original problem package `seaborn` has released since then, which may
have fixed things.

This removes some now-unnecessary suppressions from image-related
packages, and adds the new suppression for the pyzmq problem, which is
Jupyter's domain to handle.  The extra environment variable in the
images test run is to eagerly move to new default behaviour starting in
jupyter-core 6; there is no need for us to pin the package too low,
since this warning is just encouraging people to proactively test the
new behaviour, and it doesn't cause our suite problems.

* Refactor coverage CI workflow (#9361)

This relaxes the constraint on `coverage` added in #9305.  The issue
there is actually the now unmaintained `coveragepy-lcov` package is not
compatible with Coverage.py 7.0.  However, we only needed
`coveragepy-lcov` to convert Coverage's format into LCOV, which is a
feature Coverage has had itself since version 6.0.

This commit also updates some parts of the coverage workflow that were
old:

- there are new versions of the Actions `checkout` and `setup-python`,
  which swap to using Node 16 rather than Node 12, which is deprecated
  in GHA
- `grcov` is packaged and installable from `cargo` now, rather than
  needing a manual hard-coded pull from GitHub
- we can have `grcov` only keep the parts we care about immediately,
  rather than converting everything and later discarding it

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
(cherry picked from commit 7955d92)

Co-authored-by: Julien Gacon <gaconju@gmail.com>
Co-authored-by: Will Shanks <willshanks@us.ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: None Do not include in changelog stable backport potential The bug might be minimal and/or import enough to be port to stable type: qa Issues and PRs that relate to testing and code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants