updated build system and deprecated modules by mimakaev · Pull Request #76 · open2c/polychrom

mimakaev · 2025-11-10T21:59:50Z

Description

No change in functionality. Did the following:

Update the build system to pyproject.toml
Remove six and joblib dependencies to keep deps lean
deprecated polymerutils in favor of HDF5_format and issued deprecation warnings, getting ready for removing polymerutils.
Added dependensies and build deps to the build systems
Made a warning about installing openmm (it's not a dep because openmm vs openmm[cuda13] are options
Bumped max python version to 3.13 (it technically works on 3.14 pending that mp method is set to fork)

Black and better py314 compatibility are coming in the next PR.

PR Checklist

[] apply "black" to the whole codebase (black .)
[] apply isort to the codebase (isort .)
[] fun flake8 and try to resolve all the issues (work in progress!)

Copilot

Pull Request Overview

This PR migrates the build system from setup.py to pyproject.toml and removes deprecated dependencies (six, joblib, pyknotid) to streamline the package. The PR also adds an OpenMM installation warning to help users install the correct version for their system.

Key changes:

Migrated build configuration to pyproject.toml with modern Python packaging standards
Removed joblib and six dependencies, deprecating associated file format support
Added OpenMM installation warning at package import time

Reviewed Changes

Copilot reviewed 10 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
pyproject.toml	Added complete project configuration including metadata, dependencies, and build system
setup.py	Simplified to minimal Cython extension builder
requirements.txt	Removed deprecated dependencies (six, joblib, pyknotid)
polychrom/init.py	Added OpenMM installation warning
polychrom/polymerutils.py	Removed joblib/text file support, keeping only HDF5 URIs
utilities/showChainWithRasmol.py	Removed joblib import and moved polymerutils import to top
polychrom/cli/xyz	Removed unused joblib import
polychrom/cli/show	Removed unused joblib import and fixed "useage" typo
polychrom/__polymer_math.cpp	Removed unused omp.h include
docs/index.rst	Updated dependency documentation
docs/conf.py	Removed joblib from mocked modules
.gitignore	Added compiled .so files

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-10T22:02:21Z

pyproject.toml

+version = "0.1.1"
+description = "A library for polymer simulations and their analyses."
+readme = "README.md"
+requires-python = ">=3.7"


The PR description claims 'Bumped python version to 3.13', but requires-python is set to '>=3.7'. This is inconsistent. If the intention is to require Python 3.13 as the minimum version, this should be requires-python = '>=3.13'. If Python 3.7+ is the correct minimum version, the PR description should be updated to reflect that Python 3.13 is added to the classifiers (supported versions) rather than being the minimum requirement.

Suggested change

requires-python = ">=3.7"

requires-python = ">=3.13"

Copilot · 2025-11-10T22:02:22Z

polychrom/polymerutils.py

        if not full_output:
-            block = block["pos"]
+            return block["pos"]



Missing return statement when full_output=True for HDF5 files. When full_output=True, the function should return the full block dictionary but currently falls through without returning it. Add return block after line 115 or change line 115 to return block['pos'] and add an else: return block clause.

Suggested change

return block

Copilot · 2025-11-10T22:02:22Z

polychrom/polymerutils.py


    else:
-        raise ValueError("Unknown mode : %s, use h5dict, joblib, txt or pdb" % mode)
+        raise ValueError("Unknown mode : %s, use or pdb" % mode)


Error message is grammatically incorrect. 'use or pdb' should be 'use pdb' since pdb is now the only supported mode after removing txt and joblib modes.

Suggested change

raise ValueError("Unknown mode : %s, use or pdb" % mode)

raise ValueError("Unknown mode: %s. Only 'pdb' is supported." % mode)

Copilot · 2025-11-10T22:02:23Z

polychrom/polymerutils.py

    if len(blocksdat) > 0:
-        block = load(os.path.join(folder, f"block{ind}.dat"))
-    return block
+        return load(os.path.join(folder, f"block{ind}.dat"))


This line will always raise a ValueError since the load() function no longer supports .dat files (line 62 raises ValueError for non-URI filenames). The function should either handle .dat files differently or remove this deprecated code path entirely. If .dat files are still expected to be supported for backward compatibility, the load() function needs to be updated, or this should call a different function.

Copilot · 2025-11-10T22:02:23Z

docs/conf.py

    "simtk.unit.nanometer",
-    "simtk.openmm",
-    "joblib",
+    "simtk.openmm",    


Trailing whitespace after 'simtk.openmm'. Remove the extra spaces at the end of the line.

Suggested change

"simtk.openmm",

"simtk.openmm",

Copilot

Pull Request Overview

Copilot reviewed 12 out of 14 changed files in this pull request and generated 7 comments.

Comments suppressed due to low confidence (1)

polychrom/legacy/polymerutils.py:1

Import pollutes the enclosing namespace, as the imported module polychrom.polymerutils does not define 'all'.

from polychrom.polymerutils import *  # noqa: F403

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-11-10T22:29:27Z

polychrom/polymerutils.py

-    Text files in openmm-polymer format
-    joblib files in openmm-polymer format
+    """
+    A function to load a single conformation from a URI. Deprecated.


The docstring should be formatted properly. It should start with a brief one-line summary followed by a blank line, then additional details.

Suggested change

A function to load a single conformation from a URI. Deprecated.

A function to load a single conformation from a URI. Deprecated.

polychrom/polymerutils.py

Copilot · 2025-11-10T22:29:27Z

polychrom/polymerutils.py

        filename to load or a URI

    """
+    warnings.warn("polymerutils.load is deprecated. Use hdf5_format.load_URI instead.", DeprecationWarning)


The deprecation warning should include a stacklevel parameter (typically stacklevel=2) to show the warning at the caller's location rather than within the deprecated function itself, making it more useful for users.

Suggested change

warnings.warn("polymerutils.load is deprecated. Use hdf5_format.load_URI instead.", DeprecationWarning)

warnings.warn("polymerutils.load is deprecated. Use hdf5_format.load_URI instead.", DeprecationWarning, stacklevel=2)

Copilot · 2025-11-10T22:29:27Z

polychrom/polymerutils.py

+    warnings.warn(
+        "fetch_block is deprecated. Use hdf5_format.list_uris followed by hdf5_format.load_URI instead.",
+        DeprecationWarning,
+    )


The deprecation warning should include a stacklevel parameter (typically stacklevel=2) to show the warning at the caller's location rather than within the deprecated function itself, making it more useful for users.

polychrom/polymerutils.py

Copilot · 2025-11-10T22:29:28Z

polychrom/legacy/polymerutils.py

@@ -0,0 +1 @@
+from polychrom.polymerutils import *  # noqa: F403


Using wildcard imports (import *) can make it unclear what symbols are being imported and can cause namespace pollution. Consider explicitly importing the specific functions or modules needed, or at minimum document what this legacy module is expected to provide.

Suggested change

from polychrom.polymerutils import * # noqa: F403

# Legacy compatibility shim: re-exporting all public symbols from polychrom.polymerutils.

# If you know the specific symbols needed, replace the wildcard import below with explicit imports.

from polychrom.polymerutils import (

# List the specific functions, classes, or variables to import here, e.g.:

# function1,

# function2,

# Class1,

# ...

)

Copilot · 2025-11-10T22:29:28Z

setup.py

+import numpy

+# Define Cython extensions


The setup.py imports setuptools.Extension and Cython.Build.cythonize directly, but these should be available at build time from the build-system requirements. However, importing numpy directly at the module level can cause issues if numpy is not yet installed. Consider using a try-except block or ensuring numpy is installed before this import executes, or use the PEP 517 isolated build which should handle this correctly.

Suggested change

import numpy

# Define Cython extensions

# Define Cython extensions

import numpy

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

updated build system and deprecated modules

c4730e1

mimakaev requested a review from Copilot November 10, 2025 21:59

Copilot AI reviewed Nov 10, 2025

View reviewed changes

deprecate and clean up polymerutils

053ee1c

mimakaev requested a review from Copilot November 10, 2025 22:23

Copilot started reviewing on behalf of mimakaev November 10, 2025 22:24 View session

Copilot finished reviewing on behalf of mimakaev November 10, 2025 22:25

trailing spaces

029490d

Copilot AI reviewed Nov 10, 2025

View reviewed changes

mimakaev and others added 4 commits November 10, 2025 15:30

Update polychrom/polymerutils.py

021aa31

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update polychrom/polymerutils.py

a4cd9fd

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

update CI/CD

d256e97

remove python 3.9 as it fails for being too old

e64382a

mimakaev merged commit 4c9e3f8 into master Nov 10, 2025
4 checks passed

	raise ValueError("Unknown mode : %s, use or pdb" % mode)
	raise ValueError("Unknown mode: %s. Only 'pdb' is supported." % mode)

	A function to load a single conformation from a URI. Deprecated.
	A function to load a single conformation from a URI. Deprecated.

	warnings.warn("polymerutils.load is deprecated. Use hdf5_format.load_URI instead.", DeprecationWarning)
	warnings.warn("polymerutils.load is deprecated. Use hdf5_format.load_URI instead.", DeprecationWarning, stacklevel=2)

		@@ -0,0 +1 @@
		from polychrom.polymerutils import * # noqa: F403 No newline at end of file

-from polychrom.polymerutils import *  # noqa: F403
+# Legacy compatibility shim: re-exporting all public symbols from polychrom.polymerutils.
+# If you know the specific symbols needed, replace the wildcard import below with explicit imports.
+from polychrom.polymerutils import (
+    # List the specific functions, classes, or variables to import here, e.g.:
+    # function1,
+    # function2,
+    # Class1,
+    # ...
+)

Conversation

mimakaev commented Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

PR Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Nov 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

mimakaev commented Nov 10, 2025 •

edited

Loading