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

Import ABC from collections.abc for Python 3.10 compatibility. #4043

Merged
merged 1 commit into from Mar 15, 2021

Conversation

tirkarthi
Copy link
Contributor

  • Your changes are not possible to do through a plugin and relevant
    to a large audience (ideally all users of OctoPrint)
  • If your changes are large or otherwise disruptive: You have
    made sure your changes don't interfere with current development by
    talking it through with the maintainers, e.g. through a
    Brainstorming ticket
  • Your PR targets OctoPrint's devel branch if it's a completely
    new feature, or maintenance if it's a bug fix or improvement of
    existing functionality for the current stable version (no PRs
    against master or anything else please)
  • Your PR was opened from a custom branch on your repository
    (no PRs from your version of master, maintenance or devel please),
    e.g. dev/my_new_feature or fix/my_bugfix
  • Your PR only contains relevant changes: no unrelated files,
    no dead code, ideally only one commit - rebase and squash your PR
    if necessary!
  • Your changes follow the existing coding style
  • If your changes include style sheets: You have modified the
    .less source files, not the .css files (those are generated with
    lessc)
  • You have tested your changes (please state how!) - ideally you
    have added unit tests
  • You have run the existing unit tests against your changes and
    nothing broke
  • You have added yourself to the AUTHORS.md file :)

What does this PR do and why is it necessary?

Importing ABC directly from collections has been deprecated and removed in Python 3.10 . This PR updates the relevant places

How was it tested? How can it be tested by the reviewer?

Any background context you want to provide?

What are the relevant tickets if any?

Similar to #3735

Screenshots (if appropriate)

Further notes

@github-actions github-actions bot added the approved Issue has been approved by the bot or manually for further processing label Mar 11, 2021
@cp2004
Copy link
Member

cp2004 commented Mar 11, 2021

Would it be possible for these to be behind error handled blocks and against the maintenance branch, so it can go out before the 2.0.0 release dropping Python 2? That would be the ideal here, using some error handling to make it work on Python 2.7 as well.

Also, as a side note - these were finally removed in Py 3.10? It's been warning about it being removed for Py3.8, then 3.9, then 3.10....

@cp2004
Copy link
Member

cp2004 commented Mar 11, 2021

I tried to give this a test, while it probably works on Python 3.10 there's a slightly larger issue with frozendict:

Traceback (most recent call last):
  File "C:\Users\charl.TERRANCE\Development\venv310\Scripts\octoprint-script.py", line 33, in <module>
    sys.exit(load_entry_point('OctoPrint', 'console_scripts', 'octoprint')())
  File "c:\users\charl.terrance\development\octoprint core\octoprint\src\octoprint\__init__.py", line 913, in main
    from octoprint.cli import octo
  File "c:\users\charl.terrance\development\octoprint core\octoprint\src\octoprint\cli\__init__.py", line 282, in <module>
    from .dev import dev_commands  # noqa: E402
  File "c:\users\charl.terrance\development\octoprint core\octoprint\src\octoprint\cli\dev.py", line 259, in <module>
    def dev():
  File "c:\users\charl.terrance\development\venv310\lib\site-packages\click\core.py", line 1377, in decorator
    cmd = group(*args, **kwargs)(f)
  File "c:\users\charl.terrance\development\venv310\lib\site-packages\click\decorators.py", line 130, in decorator
    cmd = _make_command(f, name, attrs, cls)
  File "c:\users\charl.terrance\development\venv310\lib\site-packages\click\decorators.py", line 98, in _make_command
    return cls(
  File "c:\users\charl.terrance\development\octoprint core\octoprint\src\octoprint\cli\dev.py", line 26, in __init__
    from octoprint.util.commandline import CommandlineCaller
  File "c:\users\charl.terrance\development\octoprint core\octoprint\src\octoprint\util\__init__.py", line 27, in <module>
    import frozendict
  File "c:\users\charl.terrance\development\venv310\lib\site-packages\frozendict\__init__.py", line 16, in <module>
    class frozendict(collections.Mapping):
AttributeError: module 'collections' has no attribute 'Mapping'

It does not appear to be maintained any longer, though there are alternatives around and people trying to claim the frozen dict project to bring it up to speed.

@tirkarthi
Copy link
Contributor Author

Thanks added try/except and rebased. This was delayed couple of times and removed in python/cpython#23754 for Python 3.10

@cp2004
Copy link
Member

cp2004 commented Mar 12, 2021

That looks good to me - obviously the problem of frozendict dependency will have to be solved as well (and maybe something else) but this is at least part of it. 3.10 is still in alpha, so we have 6 months or so to come up with a solution 🙂.

I did poke about and saw there was a request for the frozendict package to be adopted on PyPi that was approved, the original maintainer is OK for it to be transferred. So once they get through the backlog it may be simple enough to upgrade - though my bets are it won't support Python 2. Maybe by then we will be closer to a working 2.0 release 😉

@foosel foosel added this to To review in PR triage Mar 15, 2021
@github-actions github-actions bot added the ci/cd Related to the CI/CD setup label Mar 15, 2021
@foosel foosel changed the base branch from devel to maintenance March 15, 2021 16:03
@github-actions github-actions bot removed the ci/cd Related to the CI/CD setup label Mar 15, 2021
@foosel
Copy link
Member

foosel commented Mar 15, 2021

Took the liberty to squash and change base to maintenance (since devel is Py3 only the Py2 fallback otherwise wouldn't really make sense either).

@foosel foosel merged commit 84a6b2b into OctoPrint:maintenance Mar 15, 2021
PR triage automation moved this from To review to Done Mar 15, 2021
foosel added a commit that referenced this pull request Mar 15, 2021
frozendict appears to be unmaintained and is incompatible to
Python 3.10+

As discussed in #4043
@foosel foosel added this to the 1.6.0 milestone Mar 29, 2021
cp2004 added a commit to cp2004/OctoPrint that referenced this pull request Jun 19, 2021
Python 3.10 introduces a backwards incompatible change around collections and collections.abcs, which tornado was impacted by.

According to Tornado's release notes https://www.tornadoweb.org/en/stable/releases/v6.0.0.html seem like there is not much else to worry about, and the devel branch has already  upgraded it.

Would be good to get this tested by someone else using Python 3.10 and some plugins, and also some Python 2 installs.

See also OctoPrint#4043 for the other collections-related changes.
foosel pushed a commit that referenced this pull request Jun 21, 2021
Python 3.10 introduces a backwards incompatible change around collections and collections.abcs, which tornado was impacted by.

According to Tornado's release notes https://www.tornadoweb.org/en/stable/releases/v6.0.0.html seem like there is not much else to worry about, and the devel branch has already  upgraded it.

Would be good to get this tested by someone else using Python 3.10 and some plugins, and also some Python 2 installs.

See also #4043 for the other collections-related changes.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Issue has been approved by the bot or manually for further processing
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants