Updates the package to support Python 3.9+ by Yinjiajun278 · Pull Request #17 · innolitics/natural-neighbor-interpolation

Yinjiajun278 · 2026-01-06T23:17:27Z

Currently, the package does not support newer python versions. I have updated it to support Python 3.14, while maintaining backward compatability to the best of my ability.
The tests have been run on my local machine (hence the non-ASCII characters support and the tox-conda integration).
I am mainly making this edit for my personal project. If there are any issues, you can let me know and I'll work on it.

johndgiese

Thank you for taking a stab at modernizing the package. I'd be happy to accept a PR, however, there are a lot of large changes that seem unrelated to Python 3.9+ support. Could you please clean out the unnecessary code and add more context regarding the changes?

johndgiese · 2026-01-06T23:35:10Z

README.rst

 - Uncomment cpplint from tox.ini and cleanup C++ code
 - Generalize the threading model (currently it uses 8 threads---one for each quadrant)

+Jan 6, 2026: Modernized to support Python 3.9+ and includes support for non-ASCII directory names


johndgiese · 2026-01-06T23:35:43Z

etup.cfg

@@ -0,0 +1,314 @@
+


This file seems like garbage

Thank you. It has been removed

johndgiese · 2026-01-06T23:36:38Z

pyproject.toml

@@ -0,0 +1,44 @@
+[build-system]


Why are we adding this vs using setup.py and setup.cfg that we already have?

I was given the advice that modern packages uses pyproject.toml to adhere to the PEP 517/518 standard. To be honest this is the first time I'm working on a library and I don't really understand why one is better than the other. I think it works either way, but I can go back to use only setup.py if you want me to.

Oh no need to revert it, I honestly didn't know that. I'll read up a bit on PEP517/518 and will get back to you.

Copilot

Pull request overview

This pull request modernizes the package to support Python 3.9+ (up to 3.14) while dropping support for older Python versions (3.5, 3.6, 3.8). The changes include migrating from distutils to setuptools, updating NumPy and C++ deprecation fixes, adding support for non-ASCII characters in Windows paths, and integrating tox-conda for better Windows development experience.

Changes:

Migrated build system from distutils to modern setuptools with pyproject.toml
Fixed NumPy and C++ deprecations for compatibility with newer versions
Added comprehensive Windows support including non-ASCII path handling and conda integration
Updated test configurations to support Python 3.9-3.14

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
tox.ini	Added Python 3.9-3.14 test environments with conda integration
setup.py	Migrated from distutils to setuptools, added Windows path handling for non-ASCII characters
setup.cfg	Updated license metadata field name to follow modern conventions
pyproject.toml	New build system configuration with project metadata
naturalneighbor/naturalneighbor.py	Fixed deprecated NumPy types (np.complex → complex, np.int → int)
naturalneighbor/init.py	Added build directory path handling for editable installs
naturalneighbor/kdtree.h	Removed deprecated std::binary_function for C++17 compatibility
tests/test_cube.py	Fixed deprecated NumPy type (np.float → np.float64)
demo/demo-requirements.txt	Unpinned dependency versions for better compatibility
run_tox_utf8.bat	New helper script for UTF-8 encoding on Windows
cpptests/test.bat	New Windows test runner with cmake detection
cpptests/CMakeLists.txt.in	Updated CMake configuration
cpptests/CMakeLists.txt	Updated CMake minimum version
README.rst	Added changelog entry for modernization
.gitignore	Added patterns for .pyd, .tox, and .png files

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

Copilot · 2026-01-30T19:11:38Z

run_tox_utf8.bat

+REM   run_tox_utf8.bat -e py312 --recreate - Recreate environment and run Python 3.12
+REM   run_tox_utf8.bat -l                 - List all available environments
+REM
+REM Available Python versions: py39, py310, py311, py312, py313, py314


The documentation in this file lists py314 as an available Python version (line 14), but Python 3.14 has not been released yet. This should be removed from the documentation to avoid confusion, or at minimum, a note should be added that py314 is for future use and may not be available yet.

Suggested change

REM Available Python versions: py39, py310, py311, py312, py313, py314

REM Available Python versions: py39, py310, py311, py312, py313

Copilot · 2026-01-30T19:11:38Z

cpptests/test.bat

+set "EXE=Debug\cpptests.exe"
+if not exist "%EXE%" (
+    echo Error: cpptests executable not found at %EXE%
+    exit /b 1
+)
+
+"%EXE%"


The batch file assumes the executable will be in the Debug directory (line 35), but CMake can generate Release builds or other configurations. If CMake is configured for a Release build or uses a different generator (like Ninja), the executable path will be different. Consider checking for the executable in multiple possible locations (Debug\cpptests.exe, Release\cpptests.exe, cpptests.exe) or using CMake's ability to run the test directly.

pyproject.toml

Copilot · 2026-01-30T19:11:39Z

tox.ini

    pytest
    numpy
 commands =
+    python -c "import subprocess, sys, os; conda_exists = subprocess.run(['conda', '--version'], capture_output=True, shell=sys.platform=='win32').returncode == 0; in_conda_env = bool(os.environ.get('CONDA_PREFIX') or os.environ.get('CONDA_DEFAULT_ENV')); conda_exists and not in_conda_env and print('WARNING: conda is available but tox-conda is not installed. Using venv instead of conda environment. Install tox-conda to use conda environments.') or None"


This complex one-liner command is difficult to read and maintain. Consider either breaking it into a separate script file or using a multi-line command with proper formatting. Additionally, the command uses boolean short-circuit evaluation with and/or for side effects, which is a Python anti-pattern and can be confusing. A simple if-statement would be more clear and maintainable.

Copilot · 2026-01-30T19:11:39Z

naturalneighbor/__init__.py

+# Add build directory to Python path for editable installs
+# This allows the compiled extension to be found when installed in editable mode
+_package_dir = os.path.dirname(os.path.abspath(__file__))
+_project_root = os.path.dirname(_package_dir)
+_build_dir = os.path.join(_project_root, 'build')
+
+# Find the platform-specific build directory (e.g., lib.win-amd64-cpython-314)
+if os.path.isdir(_build_dir):
+    for item in os.listdir(_build_dir):
+        build_lib_path = os.path.join(_build_dir, item)
+        if os.path.isdir(build_lib_path) and item.startswith('lib.'):
+            # Check if the extension exists in this build directory
+            # Extension files can have various suffixes like .cp314-win_amd64.pyd
+            ext_files = glob.glob(os.path.join(build_lib_path, 'cnaturalneighbor.*'))
+            # Filter for actual extension files (.pyd on Windows, .so on Unix)
+            ext_files = [f for f in ext_files if f.endswith(('.pyd', '.so'))]
+            if ext_files:
+                if build_lib_path not in sys.path:
+                    sys.path.insert(0, build_lib_path)
+                break


The automatic build directory path manipulation in init.py could cause unexpected import behavior in production environments. This approach modifies sys.path at import time, which can lead to:

Different behavior between editable installs and regular installs

Potential conflicts if multiple versions are installed

Harder-to-debug import issues

This logic should ideally only run in development/editable install scenarios. Consider checking if the package is installed in editable mode (e.g., by checking for a .egg-link file or similar) before modifying sys.path, or document this behavior clearly for users.

Copilot · 2026-01-30T19:11:40Z

pyproject.toml

+classifiers = [
+    "Development Status :: 4 - Beta",
+    "Intended Audience :: Developers",
+    "Intended Audience :: Science/Research",
+    "Programming Language :: Python",
+    "Programming Language :: Python :: 3",
+    "Programming Language :: Python :: 3.9",
+    "Programming Language :: Python :: 3.10",
+    "Programming Language :: Python :: 3.11",
+    "Programming Language :: Python :: 3.12",
+    "Programming Language :: Python :: 3.13",
+    "Programming Language :: Python :: Implementation :: CPython",


The pyproject.toml file is missing a classifier for Python 3.14, which is configured in tox.ini. For consistency, if Python 3.14 support is intended, it should be listed here as well. However, note that Python 3.14 has not been released yet, so this classifier should only be added once Python 3.14 is actually available.

README.rst

Copilot · 2026-01-30T19:11:40Z

setup.py

+# Check if we're running a build command (and not a metadata-only command)
+needs_build = any(cmd in sys.argv for cmd in build_commands)
+is_metadata_only = any(cmd in sys.argv for cmd in metadata_commands)

-    setup(**metadata)
+if needs_build and not is_metadata_only:


The command detection logic has a potential issue: if both a build command and a metadata command are present in sys.argv (e.g., python setup.py build clean), the current logic will skip building the extension because is_metadata_only will be True. The condition should use and not is_metadata_only or better yet, prioritize build commands. Consider changing the logic to: if needs_build and not is_metadata_only: should become if needs_build: and remove the is_metadata_only variable, since metadata commands wouldn't set needs_build=True.

Copilot · 2026-01-30T19:11:41Z

setup.py

+        import ctypes
+        from ctypes import wintypes
+
+        # GetShortPathNameW is the Unicode version that handles UTF-8 paths


The comment mentions "GetShortPathNameW is the Unicode version that handles UTF-8 paths", but this is slightly misleading. GetShortPathNameW handles Unicode (UTF-16) paths, not UTF-8. Python strings are Unicode and ctypes properly converts them to UTF-16 for the Windows API, but the comment should be more precise about the encoding involved to avoid confusion.

Suggested change

# GetShortPathNameW is the Unicode version that handles UTF-8 paths

# GetShortPathNameW is the wide-character (UTF-16) Unicode version; ctypes

# converts Python Unicode strings to UTF-16 for this Windows API call.

Copilot · 2026-01-30T19:11:41Z

setup.py

+        import ctypes
+        from ctypes import wintypes
+
+        # GetShortPathNameW is the Unicode version that handles UTF-8 paths
+        kernel32 = ctypes.windll.kernel32
+        kernel32.GetShortPathNameW.argtypes = [wintypes.LPCWSTR, wintypes.LPWSTR, wintypes.DWORD]
+        kernel32.GetShortPathNameW.restype = wintypes.DWORD
+
+        # Allocate buffer for short path (MAX_PATH = 260, but allow for longer paths)
+        buffer_size = 32767
+        buffer = ctypes.create_unicode_buffer(buffer_size)


Module 'ctypes' is imported with both 'import' and 'import from'.

Suggested change

import ctypes

from ctypes import wintypes

# GetShortPathNameW is the Unicode version that handles UTF-8 paths

kernel32 = ctypes.windll.kernel32

kernel32.GetShortPathNameW.argtypes = [wintypes.LPCWSTR, wintypes.LPWSTR, wintypes.DWORD]

kernel32.GetShortPathNameW.restype = wintypes.DWORD

# Allocate buffer for short path (MAX_PATH = 260, but allow for longer paths)

buffer_size = 32767

buffer = ctypes.create_unicode_buffer(buffer_size)

from ctypes import wintypes, windll, create_unicode_buffer

# GetShortPathNameW is the Unicode version that handles UTF-8 paths

kernel32 = windll.kernel32

kernel32.GetShortPathNameW.argtypes = [wintypes.LPCWSTR, wintypes.LPWSTR, wintypes.DWORD]

kernel32.GetShortPathNameW.restype = wintypes.DWORD

# Allocate buffer for short path (MAX_PATH = 260, but allow for longer paths)

buffer_size = 32767

buffer = create_unicode_buffer(buffer_size)

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

johndgiese

Please address the one comment and then you can merge!

johndgiese · 2026-01-30T19:26:04Z

setup.py

+                library_dirs = []
+    else:
+        # GCC/Clang compiler flags
+        extra_compile_args = ['--std=c++11', '-O3']


@Yinjiajun278 what are your thoughts on this comment? It seems to me the C++ version should be consistent across platforms.

Yinjiajun278 added 2 commits January 6, 2026 23:01

Modernised to python 3.9+

9132e43

Removed bugged file

bd5455a

johndgiese requested changes Jan 6, 2026

View reviewed changes

Deleted bugged file

06be334

Yinjiajun278 requested a review from johndgiese January 30, 2026 13:53

johndgiese requested a review from Copilot January 30, 2026 19:03

Copilot started reviewing on behalf of johndgiese January 30, 2026 19:03 View session

Copilot AI reviewed Jan 30, 2026

View reviewed changes

johndgiese and others added 3 commits January 30, 2026 14:24

Apply suggestion from @Copilot

cfb28b7

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

Apply suggestion from @Copilot

a45bbb8

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

Apply suggestion from @Copilot

8110481

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

johndgiese approved these changes Jan 30, 2026

View reviewed changes

	REM Available Python versions: py39, py310, py311, py312, py313, py314
	REM Available Python versions: py39, py310, py311, py312, py313

	# GetShortPathNameW is the Unicode version that handles UTF-8 paths
	# GetShortPathNameW is the wide-character (UTF-16) Unicode version; ctypes
	# converts Python Unicode strings to UTF-16 for this Windows API call.

Conversation

Yinjiajun278 commented Jan 6, 2026

Uh oh!

johndgiese left a comment

Choose a reason for hiding this comment

Uh oh!

johndgiese Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

johndgiese Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Yinjiajun278 Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

johndgiese Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

Yinjiajun278 Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

johndgiese Jan 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

johndgiese left a comment

Choose a reason for hiding this comment

Uh oh!

johndgiese Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants