Skip to content

Commit a2e891b

Browse files
ctruedenclaude
andcommitted
Ditch Protocol in favor of ABC class hierarchies
== Why? == The use of Protocol was done with the intention of allowing third-party duck-typed implementations of interface-style classes. But the mypy static type analyzer yields errors if type inheritance is not explicit. If third-party developers create duck-typed implementations without inheriting from Builder, they'll hit mypy errors when their implicit builder instance is used anywhere that expects the Builder type (e.g. the BuildException constructor). The only way to avoid the error is by explicitly inheriting from the Protocol anyway: class MyThirdPartyBuilder(Builder): # Still need to inherit! ... So the Protocol doesn't actually enable true structural duck-typing in practice with mypy—it still requires explicit inheritance. This defeats the whole purpose of using Protocol over ABC. The Protocol approach only provides value if: - You're not using static type checkers, OR - You mark the Protocol as @runtime_checkable and use it only for runtime isinstance() checks, OR - You're okay telling third-party developers "implement the interface but also inherit from the Protocol" (which defeats the purpose) This codebase cares about mypy compliance, so the Protocol provides no practical benefit over ABC. Therefore, we may as well use: from abc import ABC, abstractmethod class Builder(ABC): @AbstractMethod def name(self) -> str: ... # etc. This is: - Clearer in intent (you must inherit) - Simpler to understand - Identical in practice (since inheritance is required anyway for mypy) The Protocol design was well-intentioned but doesn't deliver on its promise in a type-checked codebase. == Summary of Changes == 1. Converted all Protocols to ABC: - Builder(Protocol) → Builder(ABC) - BuilderFactory(Protocol) → BuilderFactory(ABC) - Scheme(Protocol) → Scheme(ABC) - ScriptSyntax(Protocol) → ScriptSyntax(ABC) 2. Aligned Python Builder with Java's default methods: In Builder ABC, these are now concrete default methods matching Java: - rebuild() - calls delete() then build() - file() - reads file and calls content() - url() - fetches URL and calls content() - log_debug() - subscribes output/error to stdout/stderr All other methods remain abstract (require implementation). 3. Cleaned up BaseBuilder: - Removed duplicate implementations now in Builder - Made it much thinner - just stores configuration and implements required abstract methods Co-authored-by: Claude <noreply@anthropic.com>
1 parent 625f310 commit a2e891b

7 files changed

Lines changed: 77 additions & 60 deletions

File tree

src/appose/builder/__init__.py

Lines changed: 48 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,9 @@
4040
import os
4141
import shutil
4242
import sys
43+
from abc import ABC, abstractmethod
4344
from pathlib import Path
44-
from typing import Callable, Protocol
45+
from typing import Callable
4546
from urllib.request import urlopen
4647

4748
from ..environment import Environment
@@ -81,16 +82,17 @@ def __init__(
8182
self.__cause__: Exception | None = cause
8283

8384

84-
class Builder(Protocol):
85+
class Builder(ABC):
8586
"""
86-
Base protocol for all Appose environment builders.
87+
Base class for all Appose environment builders.
8788
8889
Builders are responsible for creating and configuring Appose environments.
8990
9091
The type parameter enables fluent method chaining to return the concrete
9192
builder type without requiring subclasses to override every method.
9293
"""
9394

95+
@abstractmethod
9496
def name(self) -> str:
9597
"""
9698
Returns the name of this builder (e.g., "pixi", "mamba", "uv", "custom").
@@ -100,6 +102,7 @@ def name(self) -> str:
100102
"""
101103
...
102104

105+
@abstractmethod
103106
def build(self) -> Environment:
104107
"""
105108
Builds the environment. This is the terminator method for any fluid building chain.
@@ -129,8 +132,13 @@ def rebuild(self) -> Environment:
129132
Raises:
130133
BuildException: If the build fails
131134
"""
132-
...
135+
try:
136+
self.delete()
137+
except Exception as e:
138+
raise BuildException(self, cause=e)
139+
return self.build()
133140

141+
@abstractmethod
134142
def delete(self) -> None:
135143
"""
136144
Deletes the builder's linked environment directory, if any.
@@ -140,6 +148,7 @@ def delete(self) -> None:
140148
"""
141149
...
142150

151+
@abstractmethod
143152
def wrap(self, env_dir: str | Path) -> Environment:
144153
"""
145154
Wraps an existing environment directory, detecting and using any
@@ -160,6 +169,7 @@ def wrap(self, env_dir: str | Path) -> Environment:
160169
"""
161170
...
162171

172+
@abstractmethod
163173
def env(self, key: str, value: str) -> Builder:
164174
"""
165175
Sets an environment variable to be passed to worker processes.
@@ -173,6 +183,7 @@ def env(self, key: str, value: str) -> Builder:
173183
"""
174184
...
175185

186+
@abstractmethod
176187
def env_vars(self, vars: dict[str, str]) -> Builder:
177188
"""
178189
Sets multiple environment variables to be passed to worker processes.
@@ -185,6 +196,7 @@ def env_vars(self, vars: dict[str, str]) -> Builder:
185196
"""
186197
...
187198

199+
@abstractmethod
188200
def set_name(self, env_name: str) -> Builder:
189201
"""
190202
Sets the name for the environment.
@@ -198,6 +210,7 @@ def set_name(self, env_name: str) -> Builder:
198210
"""
199211
...
200212

213+
@abstractmethod
201214
def base(self, env_dir: str | Path) -> Builder:
202215
"""
203216
Sets the base directory for the environment.
@@ -211,6 +224,7 @@ def base(self, env_dir: str | Path) -> Builder:
211224
"""
212225
...
213226

227+
@abstractmethod
214228
def channels(self, *channels: str) -> Builder:
215229
"""
216230
Adds channels/repositories to search for packages.
@@ -241,8 +255,15 @@ def file(self, path: str | Path) -> Builder:
241255
Raises:
242256
BuildException: If the file cannot be read
243257
"""
244-
...
258+
try:
259+
file_path = Path(path)
260+
with open(file_path, "r", encoding="utf-8") as f:
261+
content = f.read()
262+
return self.content(content)
263+
except Exception as e:
264+
raise BuildException(self, cause=e)
245265

266+
@abstractmethod
246267
def content(self, content: str) -> Builder:
247268
"""
248269
Specifies configuration file content to build from.
@@ -270,8 +291,14 @@ def url(self, url: str) -> Builder:
270291
Raises:
271292
BuildException: If the URL cannot be read
272293
"""
273-
...
294+
try:
295+
with urlopen(url) as response:
296+
content = response.read().decode("utf-8")
297+
return self.content(content)
298+
except Exception as e:
299+
raise BuildException(self, cause=e)
274300

301+
@abstractmethod
275302
def scheme(self, scheme: str) -> Builder:
276303
"""
277304
Explicitly specifies the scheme for the configuration.
@@ -285,6 +312,7 @@ def scheme(self, scheme: str) -> Builder:
285312
"""
286313
...
287314

315+
@abstractmethod
288316
def subscribe_progress(self, subscriber: ProgressConsumer) -> Builder:
289317
"""
290318
Registers a callback to be invoked when progress happens during environment building.
@@ -297,6 +325,7 @@ def subscribe_progress(self, subscriber: ProgressConsumer) -> Builder:
297325
"""
298326
...
299327

328+
@abstractmethod
300329
def subscribe_output(self, subscriber: Callable[[str], None]) -> Builder:
301330
"""
302331
Registers a callback to be invoked when output is generated during environment building.
@@ -309,6 +338,7 @@ def subscribe_output(self, subscriber: Callable[[str], None]) -> Builder:
309338
"""
310339
...
311340

341+
@abstractmethod
312342
def subscribe_error(self, subscriber: Callable[[str], None]) -> Builder:
313343
"""
314344
Registers a callback to be invoked when errors occur during environment building.
@@ -321,6 +351,7 @@ def subscribe_error(self, subscriber: Callable[[str], None]) -> Builder:
321351
"""
322352
...
323353

354+
@abstractmethod
324355
def flags(self, *flags: str) -> Builder:
325356
"""
326357
Adds command-line flags to be passed to the underlying tool during build operations.
@@ -347,17 +378,20 @@ def log_debug(self) -> Builder:
347378
Returns:
348379
This builder instance, for fluent-style programming
349380
"""
350-
...
381+
return self.subscribe_output(
382+
lambda msg: print(msg, file=sys.stdout, end="")
383+
).subscribe_error(lambda msg: print(msg, file=sys.stderr, end=""))
351384

352385

353-
class BuilderFactory(Protocol):
386+
class BuilderFactory(ABC):
354387
"""
355-
Factory protocol for creating builder instances.
388+
Factory base class for creating builder instances.
356389
357390
Implementations are discovered at runtime via entry points and managed by
358391
the Builders utility class.
359392
"""
360393

394+
@abstractmethod
361395
def create_builder(self) -> Builder:
362396
"""
363397
Creates a new builder instance with no configuration.
@@ -368,6 +402,7 @@ def create_builder(self) -> Builder:
368402
"""
369403
...
370404

405+
@abstractmethod
371406
def name(self) -> str:
372407
"""
373408
Returns the name of this builder (e.g., "pixi", "mamba", "custom").
@@ -377,6 +412,7 @@ def name(self) -> str:
377412
"""
378413
...
379414

415+
@abstractmethod
380416
def supports_scheme(self, scheme: str) -> bool:
381417
"""
382418
Checks if this builder supports the given scheme.
@@ -389,6 +425,7 @@ def supports_scheme(self, scheme: str) -> bool:
389425
"""
390426
...
391427

428+
@abstractmethod
392429
def priority(self) -> float:
393430
"""
394431
Returns the priority of this builder for scheme resolution.
@@ -399,6 +436,7 @@ def priority(self) -> float:
399436
"""
400437
...
401438

439+
@abstractmethod
402440
def can_wrap(self, env_dir: str | Path) -> bool:
403441
"""
404442
Checks if this builder can wrap the given existing environment directory.
@@ -412,7 +450,7 @@ def can_wrap(self, env_dir: str | Path) -> bool:
412450
...
413451

414452

415-
class BaseBuilder:
453+
class BaseBuilder(Builder):
416454
"""
417455
Base class for environment builders.
418456
Provides common implementation for the Builder protocol.
@@ -430,14 +468,6 @@ def __init__(self):
430468
self.source_content: str | None = None
431469
self.scheme: str | None = None
432470

433-
def rebuild(self) -> Environment:
434-
"""Default implementation: delete then build."""
435-
try:
436-
self.delete()
437-
except Exception as e:
438-
raise BuildException(self, cause=e)
439-
return self.build()
440-
441471
def delete(self) -> None:
442472
"""Default implementation: delete env_dir if it exists."""
443473
dir_path = self._env_dir()
@@ -479,30 +509,11 @@ def channels(self, *channels: str) -> BaseBuilder:
479509
self.channels_list.extend(channels)
480510
return self
481511

482-
def file(self, path: str | Path) -> BaseBuilder:
483-
"""Load configuration from a file."""
484-
try:
485-
file_path = Path(path)
486-
with open(file_path, "r", encoding="utf-8") as f:
487-
content = f.read()
488-
return self.content(content)
489-
except Exception as e:
490-
raise BuildException(self, cause=e)
491-
492512
def content(self, content: str) -> BaseBuilder:
493513
"""Set configuration content."""
494514
self.source_content = content
495515
return self
496516

497-
def url(self, url: str) -> BaseBuilder:
498-
"""Load configuration from a URL."""
499-
try:
500-
with urlopen(url) as response:
501-
content = response.read().decode("utf-8")
502-
return self.content(content)
503-
except Exception as e:
504-
raise BuildException(self, cause=e)
505-
506517
def scheme(self, scheme: str) -> BaseBuilder:
507518
"""Set the explicit scheme."""
508519
self.scheme = scheme
@@ -528,12 +539,6 @@ def flags(self, *flags: str) -> BaseBuilder:
528539
self.flags_list.extend(flags)
529540
return self
530541

531-
def log_debug(self) -> BaseBuilder:
532-
"""Log output and errors to stderr."""
533-
return self.subscribe_output(
534-
lambda msg: print(msg, file=sys.stdout, end="")
535-
).subscribe_error(lambda msg: print(msg, file=sys.stderr, end=""))
536-
537542
# Helper methods
538543

539544
def _env_name(self) -> str:

src/appose/builder/mamba.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535

3636
from pathlib import Path
3737

38-
from . import BaseBuilder, BuildException, Builder
38+
from . import BaseBuilder, BuildException, Builder, BuilderFactory
3939
from ..environment import Environment
4040

4141

@@ -213,7 +213,7 @@ def _create_environment(self, env_dir: Path, mamba) -> Environment:
213213
return self._create_env(base, bin_paths, launch_args)
214214

215215

216-
class MambaBuilderFactory:
216+
class MambaBuilderFactory(BuilderFactory):
217217
"""
218218
Factory for creating MambaBuilder instances.
219219
"""

src/appose/builder/pixi.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434

3535
from pathlib import Path
3636

37-
from . import BaseBuilder, BuildException, Builder
37+
from . import BaseBuilder, BuildException, Builder, BuilderFactory
3838
from ..environment import Environment
3939

4040

@@ -317,7 +317,7 @@ def _create_environment(self, env_dir: Path, pixi) -> Environment:
317317
return self._create_env(base, bin_paths, launch_args)
318318

319319

320-
class PixiBuilderFactory:
320+
class PixiBuilderFactory(BuilderFactory):
321321
"""
322322
Factory for creating PixiBuilder instances.
323323
"""

src/appose/builder/uv.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535

3636
from pathlib import Path
3737

38-
from . import BaseBuilder, BuildException, Builder
38+
from . import BaseBuilder, BuildException, Builder, BuilderFactory
3939
from ..environment import Environment
4040

4141

@@ -284,7 +284,7 @@ def _create_environment(self, env_dir: Path) -> Environment:
284284
return self._create_env(base, bin_paths, launch_args)
285285

286286

287-
class UvBuilderFactory:
287+
class UvBuilderFactory(BuilderFactory):
288288
"""
289289
Factory for creating UvBuilder instances.
290290
"""

0 commit comments

Comments
 (0)