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

Fix build against Python 3.12 #1922

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ogayot
Copy link

@ogayot ogayot commented Nov 23, 2023

Ever since Python 3.2, configparser.SafeConfigParser has been deprecated in favor of configparser.ConfigParser. An alias existed for backward compatibility but the alias was dropped from Python 3.12.

The imp module was dropped from Python 3.12, but importlib can be used as a substitute.

Furthermore, Python scripts were not consistently closing the files that they opened for writing. With earlier versions of Python, it was not a big deal. But with Python 3.12, the files appear truncated. The second patch goes through every call to open(..., mode="w") and ensures that the resource is properly cleaned up after.

The change will probably fail to build with Python 2.7.

  • Is any new functionality in tested? (This can be checked with
    make clean diff-cover or the CodeCov report that is automatically
    generated following a successful CI build.)
  • Was a spellchecker run on the source code and documentation after
    changes were made?
  • Have any changes to the command-line interface been explicitly described?
    Only backwards-compatible additions are allowed without a major version
    increment. Changing file formats also requires a major version number
    increment.
  • Have any substantial changes been documented in CHANGELOG.md? See
    keepachangelog for more details.
  • Do the changes respect streaming I/O? (Are they tested for streaming I/O?)

Ever since Python 3.2, configparser.SafeConfigParser has been deprecated
in favor of configparser.ConfigParser. An alias existed for backward
compatibility but the alias was dropped from Python 3.12.

Let us now use ConfigParser explicitly, and use .read_file() instead of
.readfp() which was dropped too.

The imp module has also been dropped, but a similar behavior can be achieved
using importlib.

Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
Python scripts under scripts/ in the source tree do not consistently
close files that they open for writing. While some of the scripts use
context managers, most of them do not (or do so inconsistently).  In
previous releases of Python, this apparently was not much of a concern.
However, Python 3.12 seems to be much less forgiving when files are not
properly closed. When running the test suite, many of the files that are
not explicitly closed appear truncated. This leads to various tests
failing or hanging.

Furthermore, khmer defines the get_file_writer() function, but it cannot
be consistently used as a context manager because it sometimes closes
the underlying file descriptor ; and sometimes does not depending on the
arguments.

Fixed by defining a new FileWriter context manager and ensuring that
each call to open() / get_file_writer() frees up resources properly.

Signed-off-by: Olivier Gayot <olivier.gayot@canonical.com>
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.

None yet

1 participant