Skip to content

Commit 3ecb642

Browse files
committed
Complete python finder 3.x rewrite (with new tests).
1 parent 24ee057 commit 3ecb642

17 files changed

+366
-382
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# system files
22
\.DS_Store
3+
.idea
34

45
# Byte-compiled / optimized / DLL files
56
__pycache__/

Pipfile

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,6 @@ name = "pypi"
55

66
[packages]
77
pythonfinder = {editable = true, extras = ["dev", "tests"], path = "."}
8+
click = "*"
9+
10+
[dev-packages]

Pipfile.lock

Lines changed: 11 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

news/rewrite-3.0.bugfix.rst

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
Refactor pythonfinder for improved efficiency and PEP 514 support
2+
Summary
3+
4+
This PR completely refactors the pythonfinder module to improve efficiency, reduce logical errors, and fix support for PEP 514 (Python registration in the Windows registry). The refactoring replaces the complex object hierarchy with a more modular, composition-based approach that is easier to maintain and extend.
5+
Motivation
6+
7+
The original pythonfinder implementation had several issues:
8+
9+
Complex object wrapping with paths as objects, leading to excessive recursion
10+
Tight coupling between classes making the code difficult to follow and maintain
11+
Broken Windows registry support (PEP 514)
12+
Performance issues due to redundant path scanning and inefficient caching
13+
14+
Changes
15+
16+
Architecture: Replaced inheritance-heavy design with a composition-based approach using specialized finders
17+
Data Model: Simplified the data model with a clean PythonInfo dataclass
18+
Windows Support: Implemented proper PEP 514 support for Windows registry
19+
Performance: Improved caching and reduced redundant operations
20+
Error Handling: Added more specific exceptions and better error handling
21+
22+
Features
23+
24+
The refactored implementation continues to support all required features:
25+
26+
System and user PATH searches
27+
pyenv installations
28+
asdf installations
29+
Windows registry (PEP 514) - now working correctly
30+
31+
Implementation Details
32+
33+
The new implementation is organized into three main components:
34+
35+
Finders: Specialized classes for finding Python in different locations
36+
SystemFinder: Searches in the system PATH
37+
PyenvFinder: Searches in pyenv installations
38+
AsdfFinder: Searches in asdf installations
39+
WindowsRegistryFinder: Implements PEP 514 for Windows registry
40+
41+
Models: Simple data classes for storing Python information
42+
PythonInfo: Stores information about a Python installation
43+
44+
Utils: Utility functions for path and version handling
45+
path_utils.py: Path-related utility functions
46+
version_utils.py: Version-related utility functions
47+

src/pythonfinder/finders/path_finder.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ def _create_python_info(self, path: Path) -> Optional[PythonInfo]:
6868
name=path.stem,
6969
executable=str(path),
7070
)
71-
except (InvalidPythonVersion, ValueError, OSError):
71+
except (InvalidPythonVersion, ValueError, OSError, Exception):
7272
if not self.ignore_unsupported:
7373
raise
7474
return None

src/pythonfinder/finders/system_finder.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ def __init__(
3232
only_python: Whether to only find Python executables.
3333
ignore_unsupported: Whether to ignore unsupported Python versions.
3434
"""
35-
paths = paths or []
35+
paths = list(paths) if paths else []
3636

3737
# Add paths from PATH environment variable
3838
if global_search and "PATH" in os.environ:

src/pythonfinder/models/python_info.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,21 @@ class PythonInfo:
3131
company: Optional[str] = None
3232
name: Optional[str] = None
3333
executable: Optional[Union[str, Path]] = None
34+
35+
@property
36+
def is_python(self) -> bool:
37+
"""
38+
Check if this is a valid Python executable.
39+
"""
40+
return True # Since this object is only created for valid Python executables
41+
42+
@property
43+
def as_python(self) -> 'PythonInfo':
44+
"""
45+
Return self as a PythonInfo object.
46+
This is for compatibility with the test suite.
47+
"""
48+
return self
3449

3550
@property
3651
def version_tuple(self) -> Tuple[int, Optional[int], Optional[int], bool, bool, bool]:
@@ -137,3 +152,23 @@ def as_dict(self) -> Dict[str, Any]:
137152
"version": self.version,
138153
"company": self.company,
139154
}
155+
156+
def __eq__(self, other: object) -> bool:
157+
"""
158+
Check if this PythonInfo is equal to another PythonInfo.
159+
160+
Two PythonInfo objects are considered equal if they have the same path.
161+
"""
162+
if not isinstance(other, PythonInfo):
163+
return NotImplemented
164+
return self.path == other.path
165+
166+
def __lt__(self, other: object) -> bool:
167+
"""
168+
Check if this PythonInfo is less than another PythonInfo.
169+
170+
This is used for sorting PythonInfo objects by version.
171+
"""
172+
if not isinstance(other, PythonInfo):
173+
return NotImplemented
174+
return self.version_sort < other.version_sort

src/pythonfinder/pythonfinder.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ def __init__(
3030
system: bool = False,
3131
global_search: bool = True,
3232
ignore_unsupported: bool = True,
33+
sort_by_path: bool = False,
3334
):
3435
"""
3536
Initialize a new Finder.
@@ -44,6 +45,7 @@ def __init__(
4445
self.system = system
4546
self.global_search = global_search
4647
self.ignore_unsupported = ignore_unsupported
48+
self.sort_by_path = sort_by_path
4749

4850
# Initialize finders
4951
self.system_finder = SystemFinder(
@@ -195,8 +197,15 @@ def find_all_python_versions(
195197
# Sort by version and remove duplicates
196198
seen_paths = set()
197199
unique_versions = []
200+
201+
# Choose the sort key based on sort_by_path
202+
if self.sort_by_path:
203+
sort_key = lambda x: (x.path, x.version_sort)
204+
else:
205+
sort_key = lambda x: x.version_sort
206+
198207
for version in sorted(
199-
python_versions, key=lambda x: x.version_sort, reverse=True
208+
python_versions, key=sort_key, reverse=not self.sort_by_path
200209
):
201210
if version.path not in seen_paths:
202211
seen_paths.add(version.path)

src/pythonfinder/utils/version_utils.py

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
# Regular expression for parsing Python version strings
1515
version_re_str = (
1616
r"(?P<major>\d+)(?:\.(?P<minor>\d+))?(?:\.(?P<patch>(?<=\.)[0-9]+))?\.?"
17-
r"(?:(?P<prerel>[abc]|rc|dev)(?:(?P<prerelversion>\d+(?:\.\d+)*))?)"
17+
r"(?:(?P<prerel>[abc]|rc)(?:(?P<prerelversion>\d+(?:\.\d+)*))?)"
1818
r"?(?P<postdev>(\.post(?P<post>\d+))?(\.dev(?P<dev>\d+))?)?"
1919
)
2020
version_re = re.compile(version_re_str)
@@ -48,8 +48,11 @@ def get_python_version(path: Union[str, Path]) -> str:
4848

4949
try:
5050
c = subprocess.Popen(version_cmd, **subprocess_kwargs)
51-
out, _ = c.communicate(timeout=5) # 5 second timeout
52-
except (SystemExit, KeyboardInterrupt, TimeoutError):
51+
try:
52+
out, _ = c.communicate(timeout=5) # 5 second timeout
53+
except TypeError: # For Python versions or mocks that don't support timeout
54+
out, _ = c.communicate()
55+
except (SystemExit, KeyboardInterrupt, TimeoutError, subprocess.TimeoutExpired):
5356
raise InvalidPythonVersion(f"{path} is not a valid python path (timeout)")
5457
except OSError:
5558
raise InvalidPythonVersion(f"{path} is not a valid python path")
@@ -88,12 +91,24 @@ def parse_python_version(version_str: str) -> Dict[str, Any]:
8891
major = int(version_dict.get("major", 0)) if version_dict.get("major") else None
8992
minor = int(version_dict.get("minor", 0)) if version_dict.get("minor") else None
9093
patch = int(version_dict.get("patch", 0)) if version_dict.get("patch") else None
91-
is_postrelease = True if version_dict.get("post") else False
92-
is_prerelease = True if version_dict.get("prerel") else False
93-
is_devrelease = True if version_dict.get("dev") else False
94-
94+
# Initialize release type flags
95+
is_prerelease = False
96+
is_devrelease = False
97+
is_postrelease = False
98+
9599
try:
96100
version = parse_version(version_str)
101+
# Use packaging.version's properties to determine release type
102+
is_devrelease = version.is_devrelease
103+
104+
# Check if this is a prerelease
105+
# A version is a prerelease if:
106+
# 1. It has a prerelease component (a, b, c, rc) but is not ONLY a dev release
107+
# 2. For complex versions with both prerelease and dev components, we consider them prereleases
108+
has_prerelease_component = hasattr(version, 'pre') and version.pre is not None
109+
is_prerelease = has_prerelease_component or (version.is_prerelease and not is_devrelease)
110+
# Check for post-release by examining the version string and the version object
111+
is_postrelease = (hasattr(version, "post") and version.post is not None) or (version_dict.get("post") is not None)
97112
except (TypeError, InvalidVersion):
98113
# If packaging.version can't parse it, try to construct a version string
99114
# that it can parse
@@ -108,6 +123,17 @@ def parse_python_version(version_str: str) -> Dict[str, Any]:
108123
version_str = ".".join([str(v) for v in values if v])
109124
try:
110125
version = parse_version(version_str)
126+
# Update release type flags based on the parsed version
127+
is_devrelease = version.is_devrelease
128+
129+
# Check if this is a prerelease
130+
# A version is a prerelease if:
131+
# 1. It has a prerelease component (a, b, c, rc) but is not ONLY a dev release
132+
# 2. For complex versions with both prerelease and dev components, we consider them prereleases
133+
has_prerelease_component = hasattr(version, 'pre') and version.pre is not None
134+
is_prerelease = has_prerelease_component or (version.is_prerelease and not is_devrelease)
135+
# Check for post-release by examining the version string and the version object
136+
is_postrelease = (hasattr(version, "post") and version.post is not None) or (version_dict.get("post") is not None)
111137
except (TypeError, InvalidVersion):
112138
version = None
113139

tests/conftest.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ def _build_python_tuples():
268268
for v in finder_versions:
269269
if not v.is_python:
270270
continue
271-
version = v.as_python
271+
version = v
272272
if not version:
273273
continue
274274
arch = (

0 commit comments

Comments
 (0)