Repository navigation
Conversation
|
Re: deprecation period: Yes, projects are mutating. :) |
de72b7d to
ebb72e3
Compare
ebb72e3 to
f5bdf9e
Compare
shinzoxD
left a comment
There was a problem hiding this comment.
copy.replace requires the replacement to have the same dynamic type as the input, but this implementation always constructs Requirement directly. On Python 3.13 at head f5bdf9e:
class CustomRequirement(Requirement):
pass
result = copy.replace(CustomRequirement(demo>=1), name=renamed)
print(type(result).__name__, isinstance(result, CustomRequirement))
# Requirement FalseThis contradicts both the copy.replace / object.__replace__ contract and the neighboring Version.__replace__, which preserves self.__class__. Please preserve the subclass type and add coverage for direct __replace__ plus copy.replace on a subclass.
Also, this is a new public API, while the project contribution guide says new features should be documented in the changelog; please add a 26.4 entry.
The existing focused suite is otherwise clean locally: 5,334 passed and 1 Python-version skip; git diff --check is clean and all 62 upstream checks are green.
notatallshaw
left a comment
There was a problem hiding this comment.
Found a couple of issues.
| raise InvalidRequirement( | ||
| f"Replacement fields do not round-trip: {str(tmp)!r}" | ||
| ) | ||
| result.specifier._prereleases = tmp.specifier._prereleases |
There was a problem hiding this comment.
I think this needs a follow up PR, or the scope of this PR needs to grow, because it's a problem with pickling as well, but specifiers can carry policy that a requirement string cannot represent:
spec = SpecifierSet([Specifier(">=1", prereleases=True)])
result = Requirement("demo").__replace__(specifier=spec)spec.filter(["1.1rc1", "1.1"]) keeps both; result.specifier.filter(...) drops 1.1rc1.
I think replacement needs to preserve a copy of spec after parsing the other fields, and Requirement.__getstate__ would also need to retain its inner policy for pickle round trips.
There was a problem hiding this comment.
Fixed with a copy.deepcopy (pickle, copy.copy and copy.deepcopy still need a followup).
Enables copy.replace() on Python 3.13+ and gives a supported way to build a modified copy of a requirement. The result is validated and normalized by re-parsing its string form, matching Version.__replace__. Assisted-by: ClaudeCode:claude-fable-5
Annotate __replace__ as returning Self and build the result with self.__class__, so a subclass gets back its own type. Assisted-by: ClaudeCode:claude-opus-5
A subclass can require arguments in __new__ or state from __init__ in __str__, so the temporary object must be a plain Requirement. The returned object still uses the subclass. Assisted-by: ClaudeCode:claude-opus-5-5
753202f to
9397e4a
Compare
The requirement string cannot represent a prereleases override on an inner Specifier, so re-parsing dropped it. Use a deep copy of the replacement SpecifierSet instead. Assisted-by: ClaudeCode:claude-opus-5-5
Let's start by providing a nice way to do this, then we can (maybe) deprecate in the future. Quite a few projects are mutating this.
Previous (available on a branch requirement-replace-and-deprecate in my fork for the future):
This was proposed in #1252. This assumes we want a deprecation period.
This enables caching
__hash__, and__hash__was always broken if you mutated this classwhile it was in a set or similar.
Added support for
__replace__as an alternative if anyone was trying to mutate these.Mutating the
extrasset inside doesn't warn. We should swap it for a frozenset when making this immutable.Assisted-by: ClaudeCode:claude-fable-5