Skip to content

Commit c043000

Browse files
committed
Copy groups and subparsers per Parser instance
The Group/Parser instances written in a class body were registered by the metaclass and shared by every instance of that Parser class: a second parse_args() — on another instance — overwrote the first instance's group and subcommand values. Parser.__init__ now clones the declared member tree (recursively for nested groups and subparser trees) so every Parser instance owns its parsed state. The class-level instances become pristine prototypes. Because every binding gets its own copy, assigning one Group instance to several attributes is now supported — each location keeps independent parsed state. The previous "referenced more than once" error is narrowed to the only unclonable shape: a cyclic group tree (detected at construction time, with a parse-time backstop for cycles wired into instance mappings after __init__). Behavior changes: - parser.group is no longer the class-body instance; code that kept a module-level reference to a Group/subparser instance must read values via the parser instance. - Reusing one Group instance across attributes works instead of raising (the copies share the prototype's title/prefix, so an explicit prefix= would still produce conflicting CLI flags). - Cycle errors fire at Parser construction instead of parse_args().
1 parent 2306799 commit c043000

6 files changed

Lines changed: 286 additions & 59 deletions

File tree

CLAUDE.md

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,11 @@ Naming follows the attribute path:
9797
- JSON/TOML: nested objects/tables
9898

9999
`Group(prefix=...)` overrides only the CLI/env segment for that group;
100-
config section names always follow the attribute path. Reusing the
101-
same Group instance in two places raises `ArgclassError` (instantiate
102-
a separate Group per attribute).
100+
config section names always follow the attribute path. Class-body
101+
Group/subparser instances are prototypes: every Parser instance works
102+
on its own copies, so one Group instance may be bound to several
103+
attributes (each binding is an independent copy). Only cyclic group
104+
trees raise `ArgclassError`.
103105

104106
### Subcommands
105107

argclass/parser.py

Lines changed: 88 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""Core parser classes for argclass."""
22

3+
import copy
34
import os
45
import weakref
56
from abc import ABCMeta
@@ -568,6 +569,52 @@ def __init__(
568569
self._defaults: Mapping[str, Any] = defaults or {}
569570

570571

572+
def _group_reuse_error(path: str) -> ArgclassError:
573+
return ArgclassError(
574+
"Group instance is referenced more than once in the parser "
575+
f"tree (current path: {path}): the group tree must not "
576+
"contain cycles.",
577+
hint="Remove the self-reference; a group cannot (directly or "
578+
"indirectly) contain itself.",
579+
)
580+
581+
582+
def _clone_group_tree(
583+
group: Group,
584+
ancestors: set[int],
585+
attr_path: tuple[str, ...],
586+
) -> Group:
587+
"""Copy ``group`` and its nested groups for one parser instance.
588+
589+
The group instances registered by the metaclass live on the class
590+
body and would otherwise be shared by every instance of the Parser
591+
class — a second ``parse_args()`` on another instance would
592+
overwrite the first one's values.
593+
594+
Because every occurrence gets its own copy, assigning one Group
595+
instance to several attributes is fine — each binding becomes an
596+
independent copy. ``ancestors`` holds the ids along the current
597+
path only, so the recursion rejects exactly the unclonable case:
598+
a group that (directly or indirectly) contains itself.
599+
"""
600+
if id(group) in ancestors:
601+
raise _group_reuse_error(".".join(attr_path))
602+
ancestors.add(id(group))
603+
try:
604+
clone = copy.copy(group)
605+
nested: dict[str, Group] = {}
606+
for child_name, child in group.__argument_groups__.items():
607+
child_clone = _clone_group_tree(
608+
child, ancestors, attr_path + (child_name,)
609+
)
610+
setattr(clone, child_name, child_clone)
611+
nested[child_name] = child_clone
612+
clone.__argument_groups__ = MappingProxyType(nested)
613+
return clone
614+
finally:
615+
ancestors.discard(id(group))
616+
617+
571618
ParserType = TypeVar("ParserType", bound="Parser")
572619

573620

@@ -737,6 +784,43 @@ def __init__(
737784
self._parser_kwargs = kwargs
738785
self._used_env_vars: set[str] = set()
739786
self._used_secret_env_vars: set[str] = set()
787+
self._materialize_members()
788+
789+
def _materialize_members(self) -> None:
790+
"""Give this parser instance its own copies of the declared
791+
groups and subparsers.
792+
793+
The instances collected by the metaclass live on the class
794+
body and are shared by every instance of this Parser class;
795+
parsing through them would let a second instance overwrite
796+
the first one's values. Copying them per instance
797+
(recursively, for nested groups and subparser trees) makes
798+
every Parser instance own its parsed state, while the
799+
class-level prototypes stay pristine.
800+
"""
801+
cls = type(self)
802+
803+
ancestors: set[int] = set()
804+
groups: dict[str, Group] = {}
805+
for name, group in cls.__argument_groups__.items():
806+
clone = _clone_group_tree(group, ancestors, (name,))
807+
setattr(self, name, clone)
808+
groups[name] = clone
809+
self.__argument_groups__ = MappingProxyType(groups)
810+
811+
subparsers: dict[str, Any] = {}
812+
for name, subparser in cls.__subparsers__.items():
813+
sub_clone = copy.copy(subparser)
814+
# The shallow copy still references the prototype's own
815+
# member clones and env-var bookkeeping; rebuild them.
816+
sub_clone._materialize_members()
817+
sub_clone._used_env_vars = set(subparser._used_env_vars)
818+
sub_clone._used_secret_env_vars = set(
819+
subparser._used_secret_env_vars
820+
)
821+
setattr(self, name, sub_clone)
822+
subparsers[name] = sub_clone
823+
self.__subparsers__ = MappingProxyType(subparsers)
740824

741825
@property
742826
def current_subparser(self) -> "AbstractParser | None":
@@ -880,15 +964,11 @@ def _fill_group(
880964
destinations: DestinationsType,
881965
visited: set[int],
882966
) -> None:
967+
# Backstop: instance trees are validated at construction time
968+
# by ``_clone_group_tree``; this catches cycles wired into the
969+
# per-instance mappings after ``__init__``.
883970
if id(group) in visited:
884-
raise ArgclassError(
885-
"Group instance is referenced more than once in the parser "
886-
f"tree (current path: {'.'.join(attr_path)}). Reusing a "
887-
"single Group instance across attributes is not supported "
888-
"because state would be shared between locations.",
889-
hint="Instantiate a new Group for each attribute, or "
890-
"subclass Group to define a dedicated type.",
891-
)
971+
raise _group_reuse_error(".".join(attr_path))
892972
visited.add(id(group))
893973

894974
section = ".".join(attr_path)

docs/groups.md

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -386,12 +386,14 @@ assert parser.database.port == 5432
386386
Rejected forms raise `ArgumentDefinitionError` immediately when the
387387
parser class is defined, with a hint suggesting the correct form.
388388

389-
### Reusing a group instance is an error
390-
391-
Passing the same `Group` instance to two different attributes raises
392-
`ArgclassError`, because that group's parsed state would otherwise be
393-
shared between locations. Instantiate a separate group per attribute,
394-
or subclass `Group` to define a dedicated type:
389+
### Reusing a group instance
390+
391+
Group instances in a class body are prototypes — every parser
392+
instance works on its own copies, so the same `Group` instance may be
393+
bound to several attributes and each binding keeps independent parsed
394+
state. Separate instances per attribute remain the clearest style
395+
(and are required when the attributes need different `title`/`prefix`
396+
options):
395397

396398
<!--- name: test_groups_nested_separate_instances --->
397399
```python

docs/pitfalls.md

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -211,11 +211,12 @@ class Parser(argclass.Parser):
211211

212212
### Reusing a Group instance across attributes
213213

214-
Each `Group` instance owns the parsed state for one location in the
215-
parser tree. Assigning the **same instance** to two attributes raises
216-
`ArgclassError`, because parsed values would otherwise be shared
217-
between both locations.
214+
Group instances written in a class body are **prototypes**: every
215+
parser instance works on its own copies. That makes assigning the
216+
same instance to several attributes safe — each binding becomes an
217+
independent copy with its own parsed state:
218218

219+
<!--- name: test_pitfall_group_instance_reuse --->
219220
```python
220221
import argclass
221222

@@ -225,26 +226,26 @@ class Credentials(argclass.Group):
225226
shared = Credentials()
226227

227228
class Parser(argclass.Parser):
228-
primary = shared # OK on its own — single binding
229-
secondary = shared # Together with `primary`, raises ArgclassError
230-
# at parse_args time (same instance bound twice)
231-
```
232-
233-
A single attribute bound to an externally-created Group is fine; the
234-
error only fires when the **same instance** is reachable through two
235-
or more attributes at parse time.
236-
237-
Create a separate instance for each attribute instead:
229+
primary = shared
230+
secondary = shared # fine: each binding gets its own copy
238231

239-
```python
240-
class Parser(argclass.Parser):
241-
primary = Credentials() # RIGHT - distinct instances
242-
secondary = Credentials()
232+
parser = Parser()
233+
parser.parse_args([
234+
"--primary-username", "alice",
235+
"--secondary-username", "bob",
236+
])
237+
assert parser.primary.username == "alice"
238+
assert parser.secondary.username == "bob"
243239
```
244240

245-
Using the same `Group` **class** twice (with different `Group()` calls)
246-
is fully supported — only sharing one already-constructed instance is
247-
disallowed.
241+
Keep in mind that the copies inherit the prototype's `title`,
242+
`description`, and `prefix`. A reused instance with an explicit
243+
`prefix=` would produce conflicting CLI flags — give each attribute
244+
its own instance when they need different options.
245+
246+
The only forbidden shape is a **cycle** — a group that (directly or
247+
indirectly) contains itself raises `ArgclassError` at parser
248+
construction time.
248249

249250
---
250251

@@ -297,14 +298,12 @@ The same applies to names of Parser API methods (`parse_args`,
297298
define on your own parser classes: an argument cannot shadow them.
298299
:::
299300

300-
:::{warning}
301-
Group and subparser attributes are **class-level singletons**: every
302-
instance of the same Parser class shares the same Group and subparser
303-
objects. A second `parse_args()` — whether on the same parser instance
304-
or on another instance of the same class — overwrites the group and
305-
subcommand values produced by the first one. When you need independent
306-
results (e.g. parsing several argv sets side by side), keep each
307-
result before the next parse, or define separate Parser classes.
301+
:::{note}
302+
Every Parser instance works on its **own copies** of the declared
303+
groups and subparsers: the instances written in the class body are
304+
just prototypes. Parsing one parser instance never affects another
305+
one, and the prototypes stay pristine. Re-parsing the *same* parser
306+
instance overwrites its previous values, as expected.
308307

309308
<!--- name: test_pitfall_shared_groups --->
310309
```python
@@ -318,11 +317,12 @@ class CLI(argclass.Parser):
318317

319318
first = CLI()
320319
second = CLI()
321-
assert first.db is second.db # the same Group object!
320+
assert first.db is not second.db # independent copies
322321

323322
first.parse_args(["--db-host", "from-first"])
324323
second.parse_args(["--db-host", "from-second"])
325-
assert first.db.host == "from-second" # overwritten by the later parse
324+
assert first.db.host == "from-first" # not affected by the later parse
325+
assert second.db.host == "from-second"
326326
```
327327
:::
328328

tests/test_audit_fixes.py

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -516,6 +516,101 @@ class B(A):
516516
assert b.LIMIT == 5
517517

518518

519+
class TestPerInstanceState:
520+
"""Fix 6: groups and subparsers are copied per Parser instance,
521+
so parsing one instance never mutates another (or the class-level
522+
prototypes)."""
523+
524+
def test_groups_independent_across_instances(self):
525+
class CLI(argclass.Parser):
526+
db = SharedDB()
527+
528+
first = CLI()
529+
second = CLI()
530+
assert first.db is not second.db
531+
532+
first.parse_args(["--db-host", "from-first"])
533+
second.parse_args(["--db-host", "from-second"])
534+
assert first.db.host == "from-first"
535+
assert second.db.host == "from-second"
536+
537+
def test_class_prototype_stays_pristine(self):
538+
class CLI(argclass.Parser):
539+
db = SharedDB()
540+
541+
prototype = CLI.__argument_groups__["db"]
542+
cli = CLI()
543+
cli.parse_args(["--db-host", "parsed"])
544+
assert cli.db is not prototype
545+
with pytest.raises(AttributeError):
546+
prototype.host
547+
548+
def test_nested_groups_independent(self):
549+
class Credentials(argclass.Group):
550+
username: str = "admin"
551+
552+
class Endpoint(argclass.Group):
553+
credentials: Credentials = Credentials()
554+
555+
class CLI(argclass.Parser):
556+
endpoint: Endpoint = Endpoint()
557+
558+
first = CLI()
559+
second = CLI()
560+
first.parse_args(["--endpoint-credentials-username", "alice"])
561+
second.parse_args(["--endpoint-credentials-username", "bob"])
562+
assert first.endpoint.credentials.username == "alice"
563+
assert second.endpoint.credentials.username == "bob"
564+
565+
def test_subparsers_independent_across_instances(self):
566+
class CLI(argclass.Parser):
567+
serve = SharedServe()
568+
569+
first = CLI()
570+
second = CLI()
571+
assert first.serve is not second.serve
572+
573+
first.parse_args(["serve", "--port", "1111"])
574+
second.parse_args(["serve", "--port", "2222"])
575+
assert first.serve.port == 1111
576+
assert second.serve.port == 2222
577+
assert first.current_subparser is first.serve
578+
assert second.current_subparser is second.serve
579+
580+
def test_subparser_groups_independent(self):
581+
class Sub(argclass.Parser):
582+
db = SharedDB()
583+
584+
class CLI(argclass.Parser):
585+
sub = Sub()
586+
587+
first = CLI()
588+
second = CLI()
589+
first.parse_args(["sub", "--db-host", "h1"])
590+
second.parse_args(["sub", "--db-host", "h2"])
591+
assert first.sub.db.host == "h1"
592+
assert second.sub.db.host == "h2"
593+
594+
def test_reparse_same_instance_overwrites(self):
595+
class CLI(argclass.Parser):
596+
db = SharedDB()
597+
598+
cli = CLI()
599+
cli.parse_args(["--db-host", "one"])
600+
cli.parse_args(["--db-host", "two"])
601+
assert cli.db.host == "two"
602+
603+
def test_group_options_preserved_on_copy(self):
604+
class CLI(argclass.Parser):
605+
db = SharedDB(prefix="dbx", defaults={"host": "preset"})
606+
607+
cli = CLI()
608+
cli.parse_args([])
609+
assert cli.db.host == "preset"
610+
cli.parse_args(["--dbx-host", "cli-value"])
611+
assert cli.db.host == "cli-value"
612+
613+
519614
class TestPathTypeUnchanged:
520615
"""Sanity: Path defaults and types keep working end to end."""
521616

0 commit comments

Comments
 (0)