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

Python version update #309

Open
wants to merge 16 commits into
base: dev
Choose a base branch
from

Conversation

MNiedzielski
Copy link

Enable tests for current Python versions.

@MNiedzielski MNiedzielski marked this pull request as ready for review March 8, 2023 05:51
@kvid
Copy link
Collaborator

kvid commented Sep 2, 2023

I'm sorry this PR hasn't got any attention since @MNiedzielski marked it ready for review several months ago. Please compare these suggested workflows and setup changes against some similar changes included in PR #251 (that also contains a huge amount of other changes). They are not equal, and I wonder if anyone can describe some advantages and disadvantages between the different choices.

  • It seems this PR suggests supporting Python 3.7, 3.8, 3.9, 3.10, and 3.11 - and test all of them in the workflow.
  • It seems PR Large scale refactoring #251 suggests supporting Python 3.8, 3.9, and 3.10; but only test 3.8 and 3.10 in the workflow.
  • It seems reasonable to exclude 3.7 as it reached end-of-life earlier this summer.
  • Why are some version numbers quoted, but not all of them in the workflows file?

@amotl
Copy link
Member

amotl commented Sep 8, 2023

Hi there,

first of all, thank you for your contribution, Mark. As @kvid outlined, GH-251 will bring in a few modernizations in this regard already. I think we should merge that one first, and then see what's left.

Which Python versions should we support? Why include these? Why exclude the others?

https://devguide.python.org/versions/ has a good orientation what to support in general. When there are other obstacles, regarding dependencies or such, with corresponding reasons to drop support for older Python versions earlier, before their official EOL date, so be it.

Why are some version numbers quoted, but not all of them in the workflows file?

Version numbers like 3.10 will be interpreted as "3.1" when not being quoted, which is wrong.

With kind regards,
Andreas.

@MNiedzielski
Copy link
Author

@kvid @amotl No strong preferences here. Agreed 3.7 should be dropped. Perhaps even 3.8 be dropped and 3.12 added to further future-proof. Also agreed GH-251 should merge first, as it ought to reduce any fixes triggered by enabling tests for newer versions.

@formatc1702
Copy link
Collaborator

Would it make sense to only support the current Python version (3.12 as of now), at least officially?

A note in the Readme could state that "WireViz requires Python 3.12. It may or may not work on other versions".

This would speed up the build process by only running it once with the proper version instead of five times (or two, as it is now), as well as keeping local development simple, without having to have all versions installed, and potentially keeping the code clean by not having to support legacy syntax (e.g. type hint syntax added in Python 3.10).

@kvid
Copy link
Collaborator

kvid commented May 6, 2024

@formatc1702 wrote:

Would it make sense to only support the current Python version (3.12 as of now), at least officially?

If we should support more than one Python version, I would look to common distributions, e.g. for Ubuntu:
The default Python3 versions for the current Ubuntu LTS distributions seems to be:

  • Python 3.8 for Ubuntu focal (20.04 LTS)
  • Python 3.10 for Ubuntu jammy (22.04 LTS)
  • Python 3.12 for Ubuntu noble (24.04 LTS)

See an updated list here: https://packages.ubuntu.com/search?keywords=python3&searchon=names&exact=1&suite=all&section=all

This would speed up the build process by only running it once with the proper version instead of five times (or two, as it is now), as well as keeping local development simple, without having to have all versions installed, and potentially keeping the code clean by not having to support legacy syntax (e.g. type hint syntax added in Python 3.10).

I agree we should limit the number of tested Python versions, and five is probably too many. However, I would consider supporting both 3.10 and 3.12 (personally, I would also like to still use 3.8 because I currently use that for other on-going projects, but I realize it'll reach end-of-life within half a year).

Is it realistic to agree on something about this to be included in #339 as the current Python versions listed in setup.py and our github workflow is a bit outdated (3.7 and 3.8 only)?

Edit: On the other hand, if we test in the workflow only the oldest supported Python version (e.g. 3.8), shouldn't we then expect it to also work in all later minor versions due to backward compatibility (except for effects from bug-fixes, etc.), or have I misunderstood something?

@kvid kvid mentioned this pull request May 6, 2024
@sb424dat
Copy link

sb424dat commented May 6, 2024

I have been using Ubuntu jammy (22.04 LTS) with no problem. I would stress full support for one version (3.8 or 3.7) and stretch to test 3.10 or 3.12. I have spun up many VM in virtual box and had no problem (that I noticed so far).

@formatc1702
Copy link
Collaborator

Perhaps if we specify compatibility with versions 3.8 to 3.12, checking new PRs/commits against 3.8 and 3.12 could be enough. For merges into master, it might make sense to enforce stricter checking for all intermediate versions as well to prevent any public blunders.

If it's possible to somehow edit the GitHub workflow config for this, I'm more than happy to integrate it.

@kvid kvid mentioned this pull request Jun 15, 2024
@ferdnyc
Copy link

ferdnyc commented Jun 16, 2024

@kvid

Edit: On the other hand, if we test in the workflow only the oldest supported Python version (e.g. 3.8), shouldn't we then expect it to also work in all later minor versions due to backward compatibility (except for effects from bug-fixes, etc.), or have I misunderstood something?

Heh. Sorry, I don't mean to laugh, but Python does not provide backwards compatibility between minor versions. They can, will, and do remove previously-deprecated features, which will often break things hard. A partial sampling of breakage in recent minor Python releases:

Removed in Python 3.12

distutils

Remove the distutils package. It was deprecated in Python 3.10 by PEP 632 “Deprecate distutils module”. For projects still using distutils and cannot be updated to something else, the setuptools project can be installed: it still provides distutils. (Contributed by Victor Stinner in gh-92584.)

importlib

Many previously deprecated cleanups in importlib have now been completed:

References to, and support for module_repr() has been removed. (Contributed by Barry Warsaw in gh-97850.)

importlib.util.set_package, importlib.util.set_loader and importlib.util.module_for_loader have all been removed. (Contributed by Brett Cannon and Nikita Sobolev in gh-65961 and gh-97850.)

Support for find_loader() and find_module() APIs have been removed. (Contributed by Barry Warsaw in gh-98040.)

importlib.abc.Finder, pkgutil.ImpImporter, and pkgutil.ImpLoader have been removed. (Contributed by Barry Warsaw in gh-98040.)

imp

The imp module has been removed. (Contributed by Barry Warsaw in gh-98040.)

io

Remove io’s io.OpenWrapper and _pyio.OpenWrapper, deprecated in Python 3.10: just use open() instead. The open() (io.open()) function is a built-in function. Since Python 3.10, _pyio.open() is also a static method. (Contributed by Victor Stinner in gh-94169.)

locale

Remove locale’s locale.format() function, deprecated in Python 3.7: use locale.format_string() instead. (Contributed by Victor Stinner in gh-94226.)

Others
  • Remove the keyfile and certfile parameters from the ftplib, imaplib, poplib and smtplib modules, and the key_file, cert_file and check_hostname parameters from the http.client module, all deprecated since Python 3.6. Use the context parameter (ssl_context in imaplib) instead. (Contributed by Victor Stinner in gh-94172.)
Removed in Python 3.11

Removed

  • Removed the @asyncio.coroutine() decorator enabling legacy generator-based coroutines to be compatible with async / await code. The function has been deprecated since Python 3.8 and the removal was initially scheduled for Python 3.10. Use async def instead. (Contributed by Illia Volochii in bpo-43216.)

  • Removed asyncio.coroutines.CoroWrapper used for wrapping legacy generator-based coroutine objects in the debug mode. (Contributed by Illia Volochii in bpo-43216.)

  • Due to significant security concerns, the reuse_address parameter of asyncio.loop.create_datagram_endpoint(), disabled in Python 3.9, is now entirely removed. This is because of the behavior of the socket option SO_REUSEADDR in UDP. (Contributed by Hugo van Kemenade in bpo-45129.)

  • Removed the binhex module, deprecated in Python 3.9. Also removed the related, similarly-deprecated binascii functions:

    • binascii.a2b_hqx()
    • binascii.b2a_hqx()
    • binascii.rlecode_hqx()
    • binascii.rldecode_hqx()

    The binascii.crc_hqx() function remains available.

    (Contributed by Victor Stinner in bpo-45085.)

Removed in Python 3.10

Removed

  • Removed special methods __int__, __float__, __floordiv__, __mod__, __divmod__, __rfloordiv__, __rmod__ and __rdivmod__ of the complex class. They always raised a TypeError. (Contributed by Serhiy Storchaka in bpo-41974.)

  • The ParserBase.error() method from the private and undocumented _markupbase module has been removed. html.parser.HTMLParser is the only subclass of ParserBase and its error() implementation was already removed in Python 3.5. (Contributed by Berker Peksag in bpo-31844.)

  • Removed the parser module, which was deprecated in 3.9 due to the switch to the new PEG parser, as well as all the C source and header files that were only being used by the old parser, including node.h, parser.h, graminit.h and grammar.h.

  • Removed the formatter module, which was deprecated in Python 3.4. It is somewhat obsolete, little used, and not tested. It was originally scheduled to be removed in Python 3.6, but such removals were delayed until after Python 2.7 EOL. Existing users should copy whatever classes they use into their code. (Contributed by Donghee Na and Terry J. Reedy in bpo-42299.)

  • Remove deprecated aliases to Collections Abstract Base Classes from the collections module. (Contributed by Victor Stinner in bpo-37324.)

Removed in Python 3.9

Removed

  • array.array: tostring() and fromstring() methods have been removed. They were aliases to tobytes() and frombytes(), deprecated since Python 3.2. (Contributed by Victor Stinner in bpo-38916.)

  • The sys.getcheckinterval() and sys.setcheckinterval() functions have been removed. They were deprecated since Python 3.2. Use sys.getswitchinterval() and sys.setswitchinterval() instead. (Contributed by Victor Stinner in bpo-37392.)

  • _dummy_thread and dummy_threading modules have been removed. These modules were deprecated since Python 3.7 which requires threading support. (Contributed by Victor Stinner in bpo-37312.)

  • The isAlive() method of threading.Thread has been removed. It was deprecated since Python 3.8. Use is_alive() instead. (Contributed by Dong-hee Na in bpo-37804.)

  • Methods getchildren() and getiterator() of classes ElementTree and Element in the ElementTree module have been removed. They were deprecated in Python 3.2. Use iter(x) or list(x) instead of x.getchildren() and x.iter() or list(x.iter()) instead of x.getiterator(). (Contributed by Serhiy Storchaka in bpo-36543.)

  • base64.encodestring() and base64.decodestring(), aliases deprecated since Python 3.1, have been removed: use base64.encodebytes() and base64.decodebytes() instead. (Contributed by Victor Stinner in bpo-39351.)

  • fractions.gcd() function has been removed, it was deprecated since Python 3.5 (bpo-22486): use math.gcd() instead. (Contributed by Victor Stinner in bpo-39350.)

  • The buffering parameter of bz2.BZ2File has been removed. Since Python 3.0, it was ignored and using it emitted a DeprecationWarning. Pass an open file object to control how the file is opened. (Contributed by Victor Stinner in bpo-39357.)

  • The encoding parameter of json.loads() has been removed. As of Python 3.1, it was deprecated and ignored; using it has emitted a DeprecationWarning since Python 3.8. (Contributed by Inada Naoki in bpo-39377)

  • with (await asyncio.lock): and with (yield from asyncio.lock): statements are not longer supported, use async with lock instead. The same is correct for asyncio.Condition and asyncio.Semaphore. (Contributed by Andrew Svetlov in bpo-34793.)

  • The asyncio.Task.current_task() and asyncio.Task.all_tasks() have been removed. They were deprecated since Python 3.7 and you can use asyncio.current_task() and asyncio.all_tasks() instead. (Contributed by Rémi Lapeyre in bpo-40967)

  • The unescape() method in the html.parser.HTMLParser class has been removed (it was deprecated since Python 3.4). html.unescape() should be used for converting character references to the corresponding unicode characters.

@ferdnyc
Copy link

ferdnyc commented Jun 16, 2024

@formatc1702

Perhaps if we specify compatibility with versions 3.8 to 3.12, checking new PRs/commits against 3.8 and 3.12 could be enough. For merges into master, it might make sense to enforce stricter checking for all intermediate versions as well to prevent any public blunders.

I'm not sure if that makes sense, because every PR is ultimately a(n eventual) prospective merge into master. It might go through dev first, but if you check it only after it's merged to dev, that just seems like a problem that could've been caught sooner.

Plus, if every merge into dev is already checked rigorously, then it's pretty safe to merge to master whenever necessary.

@ferdnyc
Copy link

ferdnyc commented Jun 16, 2024

Python does not provide backwards compatibility between minor versions. They can, will, and do remove previously-deprecated features, which will often break things hard.

That being said, running PYTHONWARNINGS=always wireviz ... even in Python 3.12 shows an impressive lack of DeprecationWarning messages; most Python projects show dozens, if not hundreds. so, WireViz's code is already pretty forward-compatible.

The only warnings that pop up are the fact that none of the files being opened by open_file_read(), open_file_write(), or open_file_append() are ever closed.

@kvid
Copy link
Collaborator

kvid commented Jun 16, 2024

@ferdnyc - Thank's a lot for filling a bit of the huge white spaces in my Python knowledge! 😃

  1. I assume many of us worry about the resource or time demand of always testing every minor version of Python in the range we want to support, and I don't know if it's really needed.

  2. In Update CI Python versions #388 you suggested "Switch to testing on 3.8 (oldest supported) and 3.12 (current release)." - so I assume you think that's enough, but please explain why, and what errors we eventually might risk not detecting by not testing the intermediate versions.

  3. If we get rid of the issues with not closing the files, would it make sense to also require no warnings in the tests?

kvid added a commit that referenced this pull request Jun 23, 2024
A number of warnings showed up when running e.g.
PYTHONWARNINGS=always python build_examples.py
See #309 (comment)

Fix: All open() calls should be in a "with open() as x" statement
to ensure closing the file when exiting the block in any way.
Otherwise, use the new file_read_text() or file_write_text() to
read or write the whole utf-8 text file and closing it.
kvid added a commit that referenced this pull request Jun 23, 2024
A number of such warnings showed up when running e.g.
PYTHONWARNINGS=always python build_examples.py
PYTHONWARNINGS=always wireviz ../../examples/demo0?.yml
See #309 (comment)

Fix: All open() calls should be in a "with open() as x" statement
to ensure closing the file when exiting the block in any way.
Otherwise, use the new file_read_text() or file_write_text() functions
to read or write the whole utf-8 text file and closing it.
kvid added a commit that referenced this pull request Jul 5, 2024
A number of such warnings showed up when running e.g.
PYTHONWARNINGS=always python build_examples.py
PYTHONWARNINGS=always wireviz ../../examples/demo0?.yml
See #309 (comment)

Fix: All open() calls should be in a "with open() as x" statement
to ensure closing the file when exiting the block in any way.
Otherwise, use the new file_read_text() or file_write_text() functions
to read or write the whole utf-8 text file and closing it.
formatc1702 pushed a commit that referenced this pull request Jul 7, 2024
A number of such warnings showed up when running e.g.
PYTHONWARNINGS=always python build_examples.py
PYTHONWARNINGS=always wireviz ../../examples/demo0?.yml
See #309 (comment)

Fix: All open() calls should be in a "with open() as x" statement
to ensure closing the file when exiting the block in any way.
Otherwise, use the new file_read_text() or file_write_text() functions
to read or write the whole utf-8 text file and closing it.
kvid pushed a commit that referenced this pull request Oct 14, 2024
Running 6 different Python versions (3.7 to 3.12) in parallel now.
NOTE: This is in conflict with #309, but can be resolved easily in a later PR.

GitHub Actions require an update:
- actions/upload-artifact@v3 is scheduled for deprecation on November
30, 2024.
- Similarly, v1/v2 are scheduled for deprecation on June 30, 2024. 
- Updating this comes with a breaking change in upload-artifact@v4:

Uploading to the same named Artifact multiple times.

Due to how Artifacts are created in this new version, it is no longer
possible to upload to the same named Artifact multiple times. You must
either split the uploads into multiple Artifacts with different names,
or only upload once. Otherwise you will encounter an error.

The artifact .zip files therefore have the python version added to
their name.
kvid pushed a commit to kvid/WireViz that referenced this pull request Oct 14, 2024
Running 6 different Python versions (3.7 to 3.12) in parallel now.
NOTE: This is in conflict with wireviz#309, but can be resolved easily in a later PR.

GitHub Actions require an update:
- actions/upload-artifact@v3 is scheduled for deprecation on November
30, 2024.
- Similarly, v1/v2 are scheduled for deprecation on June 30, 2024. 
- Updating this comes with a breaking change in upload-artifact@v4:

Uploading to the same named Artifact multiple times.

Due to how Artifacts are created in this new version, it is no longer
possible to upload to the same named Artifact multiple times. You must
either split the uploads into multiple Artifacts with different names,
or only upload once. Otherwise you will encounter an error.

The artifact .zip files therefore have the python version added to
their name.
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.

8 participants