pi-thon 3.14 fixes #75
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR updates the polychrom codebase for Python 3.14 compatibility, focusing on multiprocessing method changes (fork → forkserver), addressing deprecation warnings, and applying code formatting with black/isort.
Key Changes:
- Updated CI/CD to test against Python 3.14 and recent macOS versions
- Refactored contactmaps module for spawn/forkserver multiprocessing compatibility (lambda functions → named functions)
- Fixed NumPy deprecation (
np.in1d→np.isin) and improved type checking patterns - Applied consistent code formatting across the codebase
Reviewed Changes
Copilot reviewed 24 out of 25 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/pythonapp.yml |
Added Python 3.14 and macOS testing matrix |
polychrom/contactmaps.py |
Refactored for multiprocessing compatibility: moved lambdas to module-level functions, added default function parameters |
tests/test_contactmaps.py |
Replaced lambda with named load_function for spawn/forkserver compatibility; added module-level test data |
polychrom/forces.py |
Updated cylindrical_confinement logic and type hints; contains a bug with bottom=0 handling |
polychrom/forcekits.py |
Added explicit error for unsupported nonbonded forces (potential breaking change) |
tests/test_polymer_analyses.py |
Fixed NumPy deprecation: np.in1d → np.isin |
tests/test_io.py |
Updated to use new load_URI API instead of deprecated fetch_block |
polychrom/polymer_analyses.py |
Added TYPE_CHECKING pattern for better Cython import handling |
polychrom/param_units.py |
Reformatted docstrings with black (r""" prefix) |
polychrom/hdf5_format.py |
Changed type() to isinstance() for better practices |
polychrom/legacy/forces.py |
Reordered imports and changed to isinstance() |
| Various files | Import reordering (isort), spacing fixes, removed extra blank lines (black formatting) |
tests/test_configurations.py |
Removed entire test file for old starting conformation generator |
| Examples | Import reordering and formatting improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
polychrom/forces.py
Outdated
| bottom = 0 if bottom is True else bottom | ||
|
|
||
| if bottom is not None: | ||
| if bottom: |
There was a problem hiding this comment.
The logic change here introduces a bug. When bottom=0 is passed, the condition if bottom: (line 734) will evaluate to False because 0 is falsy in Python, causing the code to use the wrong branch.
The original code correctly used if bottom is not None: which would handle bottom=0 properly. The fix should be:
bottom = 0 if bottom is True else bottom
if bottom is not None:
# ... existing code for bottom confinement| if bottom: | |
| if bottom is not None: |
| else: | ||
| raise ValueError("Nonbonded force {} does not support exceptions or exclusions.".format(nb_force.name)) |
There was a problem hiding this comment.
This change alters the behavior when except_bonds=True and the nonbonded force doesn't support exceptions or exclusions. Previously, the code would silently continue without adding exceptions. Now it raises a ValueError.
While this is likely an improvement (making the error explicit rather than silent), it could break existing code that relies on the previous behavior. Consider documenting this breaking change or verifying that all nonbonded forces in use support either exceptions or exclusions.
| @@ -1 +1 @@ | |||
| from polychrom.polymerutils import * # noqa: F403 | |||
| from polychrom.polymerutils import * # noqa: F403 | |||
There was a problem hiding this comment.
Import pollutes the enclosing namespace, as the imported module polychrom.polymerutils does not define 'all'.
| from polychrom.polymerutils import * # noqa: F403 | |
| # TODO: Replace the following line with explicit imports of only the required names. | |
| # For example: from polychrom.polymerutils import foo, bar | |
| # from polychrom.polymerutils import * # noqa: F403 |
| from mirnylib.numutils import zoomArray | ||
| from openmmlib import polymerutils | ||
|
|
||
| import matplotlib |
There was a problem hiding this comment.
Import of 'matplotlib' is not used.
| import matplotlib |
| from mirnylib.numutils import completeIC, coarsegrain | ||
| from mirnylib.h5dict import h5dict | ||
| from mirnylib.numutils import coarsegrain, completeIC, zoomArray | ||
| from mirnylib.plotting import nicePlot |
There was a problem hiding this comment.
Import of 'nicePlot' is not used.
| from mirnylib.plotting import nicePlot |
| import warnings | ||
| from collections.abc import Iterable | ||
| from typing import Optional, Dict | ||
| from typing import Dict, Optional |
There was a problem hiding this comment.
Import of 'Dict' is not used.
Import of 'Optional' is not used.
| from typing import Dict, Optional |
|
unused imports and other things will be addressed in a next PR |
Description
This PR contains fixes for the python 3.14 switching from fork to forkserver multiprocessing method, addresses some deprecation warnings, and reformats things with black.
spawnandforkservermultiprocessing (latest default in 3.14)PR Checklist
black .)isort .)flake8and try to resolve all the issues (work in progress!)