Skip to content

Commit 1e84c5d

Browse files
committed
refactor: version handling with new Version class
- Added `bughog/version_control/version.py` with the `Version` class. - Refactored `ExperimentResult` and `Executable` to use `Version` objects. - Updated `MongoDB` to handle `Version` serialization and deserialization. - Added the `packaging` library as a dependency in `pyproject.toml`. - Added tests for the `Version` class and updated existing tests.
1 parent adbfa74 commit 1e84c5d

8 files changed

Lines changed: 169 additions & 37 deletions

File tree

bughog/database/mongo/mongodb.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from pymongo.errors import ServerSelectionTimeoutError
1212

1313
from bughog.evaluation.experiment_result import ExperimentResult
14+
from bughog.version_control.version import Version
1415
from bughog.parameters import (
1516
DatabaseParameters,
1617
EvaluationParameters,
@@ -147,7 +148,7 @@ def store_result(self, params: ExperimentParameters, result: ExperimentResult):
147148
subject_config = params.subject_configuration
148149
collection = self.__get_data_collection(subject_config)
149150
query = {
150-
'subject_version': result.executable_version,
151+
'subject_version': str(result.executable_version) if result.executable_version else None,
151152
'executable_origin': result.executable_origin,
152153
'padded_subject_version': result.padded_subject_version,
153154
'subject_config': subject_config.subject_setting,
@@ -183,7 +184,7 @@ def get_result(self, params: ExperimentParameters) -> Optional[ExperimentResult]
183184
doc = collection.find_one(query)
184185
if doc:
185186
return ExperimentResult(
186-
doc['subject_version'],
187+
Version(doc['subject_version']),
187188
doc['executable_origin'],
188189
doc['state'],
189190
doc['result']['raw'],

bughog/evaluation/experiment_result.py

Lines changed: 11 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
import re
21
from dataclasses import dataclass
3-
from typing import Optional
2+
3+
from bughog.version_control.version import Version
44

55

66
@dataclass(frozen=True)
77
class ExperimentResult:
8-
executable_version: Optional[str]
9-
executable_origin: Optional[str]
8+
executable_version: Version | None
9+
executable_origin: str | None
1010
state: dict
1111
raw_results: dict
1212
result_variables: set[tuple[str, str]]
@@ -17,7 +17,7 @@ def is_reproduced(self) -> bool:
1717
return self.poc_is_reproduced(self.result_variables)
1818

1919
@staticmethod
20-
def poc_is_reproduced(result_variables: Optional[set[tuple[str, str]]]) -> bool:
20+
def poc_is_reproduced(result_variables: set[tuple[str, str]] | None) -> bool:
2121
if result_variables is None:
2222
return False
2323
for key, value in result_variables:
@@ -26,7 +26,7 @@ def poc_is_reproduced(result_variables: Optional[set[tuple[str, str]]]) -> bool:
2626
return False
2727

2828
@staticmethod
29-
def poc_passed_sanity_check(result_variables: Optional[set[tuple[str, str]]]) -> bool:
29+
def poc_passed_sanity_check(result_variables: set[tuple[str, str]] | None) -> bool:
3030
if result_variables is None:
3131
return False
3232
for key, value in result_variables:
@@ -35,7 +35,7 @@ def poc_passed_sanity_check(result_variables: Optional[set[tuple[str, str]]]) ->
3535
return False
3636

3737
@staticmethod
38-
def poc_is_dirty(result_variables: Optional[set[tuple[str, str]]]) -> bool:
38+
def poc_is_dirty(result_variables: set[tuple[str, str]] | None) -> bool:
3939
"""
4040
Returns whether the poc is dirty: it is not reproduced and the sanity check did not succeed.
4141
"""
@@ -48,32 +48,14 @@ def padded_subject_version(self) -> str:
4848
"""
4949
Returns a zero-padded version string derived from the executable's version,
5050
suitable for lexicographic comparison.
51-
52-
Each dot-separated numeric segment is left-padded with zeros to 4 digits.
53-
A trailing build-metadata suffix (e.g. the '-<hash>' in '0.0.1-abc123f')
54-
is stripped from the last segment before padding and then re-attached, so
55-
both 'M.m.p' and 'M.m.p-hash' version formats are handled uniformly.
56-
The result for '0.0.1-abc123f' would be '0000.0000.0001-abc123f'.
57-
58-
Raises ValueError if executable_version is None or does not match the
59-
expected format (1-4 digit dot-separated segments with an optional
60-
trailing '-<suffix>').
6151
"""
62-
if self.executable_version is None or not re.fullmatch(r'\d{1,4}(\.\d{1,4})*(-\w+)?', self.executable_version):
63-
raise ValueError(f"Unsupported version format: '{self.executable_version}'")
64-
padding_target = 4
65-
padded_version = []
66-
for sub in self.executable_version.split('.'):
67-
numeric, _, suffix = sub.partition('-')
68-
padded = '0' * (padding_target - len(numeric)) + numeric
69-
if suffix:
70-
padded += '-' + suffix
71-
padded_version.append(padded)
72-
return '.'.join(padded_version)
52+
if self.executable_version is None:
53+
raise ValueError('executable_version is None')
54+
return self.executable_version.padded()
7355

7456
def to_dict(self) -> dict:
7557
return {
76-
'executable_version': self.executable_version,
58+
'executable_version': str(self.executable_version) if self.executable_version else None,
7759
'executable_origin': self.executable_origin,
7860
'state': self.state,
7961
'raw_results': self.raw_results,

bughog/subject/executable.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@
66
import time
77
from abc import ABC, abstractmethod
88
from enum import Enum, auto, unique
9-
from typing import Optional
109

1110
from bughog import util
1211
from bughog.evaluation.collectors.logs import LogCollector
1312
from bughog.evaluation.file_structure import Folder
1413
from bughog.parameters import SubjectConfiguration
1514
from bughog.version_control.state.base import State
15+
from bughog.version_control.version import Version
1616

1717
logger = logging.getLogger(__name__)
1818

@@ -32,7 +32,7 @@ def __init__(self, config: SubjectConfiguration, state: State) -> None:
3232
self._runtime_env_vars = {}
3333
self._runtime_args = []
3434
self.__version = None
35-
self.__process: Optional[subprocess.Popen] = None
35+
self.__process: subprocess.Popen | None = None
3636

3737
# #
3838
# TO BE IMPLEMENT BY EVERY EVALUATION SUBJECT EXECUTABLE
@@ -147,10 +147,11 @@ def is_ready_for_use(self) -> bool:
147147
return os.path.isfile(self.executable_path) and self.version is not None
148148

149149
@property
150-
def version(self) -> Optional[str]:
150+
def version(self) -> Version | None:
151151
if self.__version is None:
152152
try:
153-
self.__version = self._get_version()
153+
version_str = self._get_version()
154+
self.__version = Version(version_str)
154155
except Exception:
155156
logger.error(f'Could not retrieve version for {self.state}', exc_info=True)
156157
return None
@@ -212,7 +213,7 @@ def unstage(self):
212213
elif os.path.isdir(self.staging_folder):
213214
shutil.rmtree(self.staging_folder)
214215

215-
def run(self, experiment_specific_params: list[str], cwd: Optional[Folder] = None):
216+
def run(self, experiment_specific_params: list[str], cwd: Folder | None = None):
216217
"""
217218
Runs the executable with the given arguments, and kills it after the given timeout.
218219
"""

bughog/version_control/version.py

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
from functools import total_ordering
2+
from typing import Any
3+
4+
from packaging.version import parse
5+
6+
7+
@total_ordering
8+
class Version:
9+
"""
10+
A wrapper around packaging.version.Version to provide BugHog-specific
11+
logic like 'selectable_version' and consistent padding.
12+
"""
13+
14+
def __init__(self, version_str: str):
15+
self._original_str = version_str
16+
# Handle formats like "0.0.1-<hash>" (Servo).
17+
# PEP 440 local versions use '+' (e.g. 0.0.1+hash).
18+
parsed_str = version_str
19+
if '-' in version_str:
20+
base, suffix = version_str.split('-', 1)
21+
parsed_str = f'{base}+{suffix}'
22+
23+
self._v = parse(parsed_str)
24+
25+
@property
26+
def major(self) -> int:
27+
return self._v.major
28+
29+
@property
30+
def minor(self) -> int:
31+
return self._v.minor
32+
33+
@property
34+
def patch(self) -> int:
35+
return self._v.micro
36+
37+
@property
38+
def is_pre_release(self) -> bool:
39+
return self.major == 0
40+
41+
@property
42+
def selectable_version(self) -> str:
43+
"""
44+
Returns the version string that should be used for selection in the UI/API.
45+
- For major versions >= 1, returns the major version (e.g. "100").
46+
- For major versions == 0 and minor >= 1, returns major.minor (e.g. "0.1").
47+
- For major versions == 0 and minor == 0, returns major.minor.patch (e.g. "0.0.1").
48+
"""
49+
if self.major >= 1:
50+
return str(self.major)
51+
if self.minor >= 1:
52+
return f'{self.major}.{self.minor}'
53+
return f'{self.major}.{self.minor}.{self.patch}'
54+
55+
def padded(self, segment_length: int = 4) -> str:
56+
"""
57+
Returns a zero-padded version string suitable for lexicographic comparison.
58+
Matches the logic in ExperimentResult.padded_subject_version.
59+
"""
60+
padded_segments = []
61+
for s in self._v.release:
62+
s_str = str(s)
63+
if len(s_str) > segment_length:
64+
raise ValueError(f"Version segment '{s_str}' exceeds maximum length of {segment_length}")
65+
padded_segments.append(s_str.zfill(segment_length))
66+
67+
padded_base = '.'.join(padded_segments)
68+
69+
if self._v.local:
70+
return f'{padded_base}-{self._v.local}'
71+
return padded_base
72+
73+
def __eq__(self, other: Any) -> bool:
74+
if not isinstance(other, Version):
75+
return NotImplemented
76+
return self._v == other._v
77+
78+
def __lt__(self, other: Any) -> bool:
79+
if not isinstance(other, Version):
80+
return NotImplemented
81+
return self._v < other._v
82+
83+
def __repr__(self) -> str:
84+
return f"Version('{self._original_str}')"
85+
86+
def __str__(self) -> str:
87+
return self._original_str

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ dependencies = [
88
"flask-sock==0.7.0",
99
"flatten-dict==0.4.2",
1010
"gunicorn==25.1.0",
11+
"packaging>=26.0",
1112
"pillow==12.1.1",
1213
"pyautogui==0.9.54",
1314
"pydantic>=2.12.5",

test/states/test_experiment_result.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import pytest
22

33
from bughog.evaluation.experiment_result import ExperimentResult
4+
from bughog.version_control.version import Version
45

56

67
def _make_result(result_variables, executable_version='100.0.1.1'):
78
return ExperimentResult(
8-
executable_version=executable_version,
9+
executable_version=Version(executable_version) if executable_version else None,
910
executable_origin='public',
1011
state={'type': 'commit', 'commit_nb': 1},
1112
raw_results={},

test/states/test_version.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
import pytest
2+
from bughog.version_control.version import Version
3+
4+
5+
def test_version_parsing():
6+
v = Version('100.0.5')
7+
assert v.major == 100
8+
assert v.minor == 0
9+
assert v.patch == 5
10+
assert not v.is_pre_release
11+
12+
13+
def test_version_pre_release():
14+
v = Version('0.1.2')
15+
assert v.major == 0
16+
assert v.minor == 1
17+
assert v.patch == 2
18+
assert v.is_pre_release
19+
20+
21+
def test_servo_version():
22+
v = Version('0.0.1-abc123f')
23+
assert v.major == 0
24+
assert v.minor == 0
25+
assert v.patch == 1
26+
# packaging.version local segment will contain the hash
27+
assert str(v) == '0.0.1-abc123f'
28+
29+
30+
def test_selectable_version_major():
31+
assert Version('100.0.5').selectable_version == '100'
32+
assert Version('1.2.3').selectable_version == '1'
33+
34+
35+
def test_selectable_version_pre_release():
36+
assert Version('0.1.2').selectable_version == '0.1'
37+
# Now includes patch when minor is also 0
38+
assert Version('0.0.1').selectable_version == '0.0.1'
39+
40+
41+
def test_version_comparison():
42+
assert Version('100.0.0') > Version('99.0.0')
43+
assert Version('0.1.2') > Version('0.1.1')
44+
assert Version('0.1.2') < Version('1.0.0')
45+
assert Version('100.0.1') == Version('100.0.1')
46+
47+
48+
def test_padded_version():
49+
assert Version('100.0.5').padded() == '0100.0000.0005'
50+
assert Version('0.1.2').padded() == '0000.0001.0002'
51+
# Servo style
52+
assert Version('0.0.1-abc123f').padded() == '0000.0000.0001-abc123f'
53+
54+
55+
def test_invalid_version():
56+
with pytest.raises(Exception):
57+
Version('not-a-version')

uv.lock

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

0 commit comments

Comments
 (0)