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

Pyqt5 to pyqt6 migration #37

Merged

Conversation

RaghavaAlajangi
Copy link
Contributor

@RaghavaAlajangi RaghavaAlajangi commented Dec 3, 2024

This PR aims to implement migration from pyqt5 to pyqt6. This is described in the issue #35.

  • Replace all the pyqt5 objects with pyqt6
  • Run tests locally
  • Update the CHANGELOG
  • Passed GitHub CICD

assert np.allclose(parmsf["E1"].value, 15388.787369767488,
atol=10) # this fit is not very stable
# assert np.allclose(parmsf["E1"].value, 15388.787369767488,
# atol=10) # this fit is not very stable
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I commented this assertion because it keeps failing. I don't know how to fix it.

Copy link
Member

Choose a reason for hiding this comment

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

That is fine. You can uncomment it again. I will deal with it later.

@RaghavaAlajangi
Copy link
Contributor Author

Hi @paulmueller
Please review this PR and let me know if any changes need to be made.

Copy link

codecov bot commented Dec 4, 2024

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@paulmueller
Copy link
Member

I am having problems running the tests with Python 3.12:

pytest --sw tests/
===================================================================================================== test session starts =====================================================================================================
platform linux -- Python 3.12.3, pytest-8.3.4, pluggy-1.5.0
PyQt6 6.7.1 -- Qt runtime 6.7.3 -- Qt compiled 6.7.1
rootdir: /tmp/PyJibe
configfile: pyproject.toml
plugins: qt-4.4.0
collected 29 items                                                                                                                                                                                                            
stepwise: no previously failed tests, not skipping.

tests/test_fd_base.py .F

========================================================================================================== FAILURES ===========================================================================================================
_________________________________________________________________________________________________ test_clear_and_verify_data __________________________________________________________________________________________________

qtbot = <pytestqt.qtbot.QtBot object at 0x7ee885d296a0>

    def test_clear_and_verify_data(qtbot):
        main_window = pyjibe.head.PyJibe()
>       main_window.load_data(files=make_directory_with_data())

tests/test_fd_base.py:23: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
pyjibe/head/main.py:148: in load_data
    self.add_subwindow(aclass, flist)
pyjibe/head/main.py:153: in add_subwindow
    inst = aclass(sub)
pyjibe/fd/main.py:37: in __init__
    uic.loadUi(path_ui, self)
.env/lib/python3.12/site-packages/PyQt6/uic/load_ui.py:86: in loadUi
    return DynamicUILoader(package).loadUi(uifile, baseinstance)
.env/lib/python3.12/site-packages/PyQt6/uic/Loader/loader.py:62: in loadUi
    return self.parse(filename)
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:1014: in parse
    self._handle_widget(ui_file.widget)
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:842: in _handle_widget
    self.traverseWidgetTree(el)
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:818: in traverseWidgetTree
    handler(self, child)
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:512: in createLayout
    self.traverseWidgetTree(elem)
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:818: in traverseWidgetTree
    handler(self, child)
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:555: in handleItem
    self.traverseWidgetTree(elem)
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:818: in traverseWidgetTree
    handler(self, child)
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:280: in createWidget
    self.traverseWidgetTree(elem)
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:818: in traverseWidgetTree
    handler(self, child)
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:512: in createLayout
    self.traverseWidgetTree(elem)
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:818: in traverseWidgetTree
    handler(self, child)
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:555: in handleItem
    self.traverseWidgetTree(elem)
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:818: in traverseWidgetTree
    handler(self, child)
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:280: in createWidget
    self.traverseWidgetTree(elem)
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:818: in traverseWidgetTree
    handler(self, child)
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:271: in createWidget
    self.stack.push(self._setupObject(widget_class, parent, elem))
.env/lib/python3.12/site-packages/PyQt6/uic/uiparser.py:233: in _setupObject
    obj = self.factory.createQtObject(class_name, object_name,
.env/lib/python3.12/site-packages/PyQt6/uic/objcreator.py:119: in createQtObject
    return self._cpolicy.instantiate(ctor, object_name, ctor_args,
.env/lib/python3.12/site-packages/PyQt6/uic/Loader/qobjectcreator.py:145: in instantiate
    return ctor(*ctor_args, **ctor_kwargs)
pyjibe/fd/widget_plot_fd.py:15: in __init__
    self.mpl_curve.add_toolbar(self)
pyjibe/fd/mpl_indent.py:59: in add_toolbar
    self.toolbar = custom_widgets.NavigationToolbarIndent(
pyjibe/head/custom_widgets/mpl_navigation_toolbar_icons.py:61: in __init__
    super(NavigationToolbarIndent, self).__init__(*args, **kwargs)
pyjibe/head/custom_widgets/mpl_navigation_toolbar_icons.py:17: in __init__
    super(NavigationToolbarCustom, self).__init__(*args, **kwargs)
.env/lib/python3.12/site-packages/matplotlib/backends/backend_qt.py:654: in __init__
    a = self.addAction(self._icon(image_file + '.png'),
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <pyjibe.head.custom_widgets.mpl_navigation_toolbar_icons.NavigationToolbarIndent object at 0x7ee87581dc70>, name = 'home_large.png', color = None

    def _icon(self, name, color=None):
        """Override matplotlibs `_icon` function to get custom icons"""
        name = name.replace('.png', '_large.png')
        impath = str(cbook._get_data_path('images', name))
        if not os.path.exists(impath):
            imdir = pkg_resources.resource_filename("pyjibe", "img")
            impath = os.path.join(imdir, name)
        pm = QtGui.QPixmap(impath)
        pm.setDevicePixelRatio(self.devicePixelRatioF() or 1)
        if self.palette().color(self.backgroundRole()).value() < 128:
            icon_color = self.palette().color(self.foregroundRole())
            mask = pm.createMaskFromColor(QtGui.QColor('black'),
>                                         QtCore.Qt.MaskOutColor)
E           AttributeError: type object 'Qt' has no attribute 'MaskOutColor'

pyjibe/head/custom_widgets/mpl_navigation_toolbar_icons.py:31: AttributeError
====================================================================================================== warnings summary =======================================================================================================
.env/lib/python3.12/site-packages/afmformats/formats/fmt_jpk/jpk_meta.py:3
  /tmp/PyJibe/.env/lib/python3.12/site-packages/afmformats/formats/fmt_jpk/jpk_meta.py:3: DeprecationWarning: pkg_resources is deprecated as an API. See https://setuptools.pypa.io/en/latest/pkg_resources.html
    from pkg_resources import resource_filename

.env/lib/python3.12/site-packages/pkg_resources/__init__.py:3149
  /tmp/PyJibe/.env/lib/python3.12/site-packages/pkg_resources/__init__.py:3149: DeprecationWarning: Deprecated call to `pkg_resources.declare_namespace('mpl_toolkits')`.
  Implementing implicit namespace packages (as specified in PEP 420) is preferred to `pkg_resources.declare_namespace`. See https://setuptools.pypa.io/en/latest/references/keywords.html#keyword-namespace-packages
    declare_namespace(pkg)

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=================================================================================================== short test summary info ===================================================================================================
FAILED tests/test_fd_base.py::test_clear_and_verify_data - AttributeError: type object 'Qt' has no attribute 'MaskOutColor'
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! Interrupted: Test failed, continuing from this test next run. !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
=========================================================================================== 1 failed, 1 passed, 2 warnings in 1.84s ===========================================================================================

I have the feeling that this is related to pkg_resources.
Thus, I think fixing #33 should be done before migrating to PyQt6. The migration to PyQt6 should not break support for Python 3.12.

@RaghavaAlajangi
Copy link
Contributor Author

RaghavaAlajangi commented Dec 4, 2024

I can also work on this issue and create a new PR if you want.

@RaghavaAlajangi
Copy link
Contributor Author

Hi Paul,
as you mentioned, I uncommented the assertion in the test function and merged the importlib.resources changes. Can you review these changes?

@paulmueller paulmueller changed the base branch from master to ci December 13, 2024 14:01
@paulmueller paulmueller changed the base branch from ci to master December 13, 2024 14:01
@@ -1,3 +1,5 @@
0.16.0
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to increment the minor version. Put it under 0.15.10 which is not released yet.

Copy link
Member

Choose a reason for hiding this comment

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

on second thought, pyqt6 is a major change, so it's good this way 👍

@paulmueller paulmueller merged commit 96f38cd into AFM-analysis:master Dec 16, 2024
4 of 5 checks passed
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