-
-
Notifications
You must be signed in to change notification settings - Fork 128
Add __replace__ magic method to BaseContainer for copy.replace() support #2093
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Add __replace__ magic method to BaseContainer for copy.replace() support #2093
Conversation
5e23f16
to
3169b17
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
tests/test_primitives/test_container/test_base_container/test_replace.py
Outdated
Show resolved
Hide resolved
tests/test_primitives/test_container/test_base_container/test_replace.py
Outdated
Show resolved
Hide resolved
tests/test_primitives/test_container/test_base_container/test_replace.py
Outdated
Show resolved
Hide resolved
assert copy.replace(container) is container # type: ignore[attr-defined] | ||
|
||
new_value = 'new_value' | ||
new_container = copy.replace(container, _inner_value=new_value) # type: ignore[attr-defined] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with copy.replace
is that it breaks typing :(
It does not know that copy.replace(Success(1), 'a')
will be Result[str, Any]
and not Result[int, Any]
We need a typed wrapper around it for our containers. With HKT support.
41b4bfb
to
288301e
Compare
288301e
to
77b5252
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sobolevn , thanks for your review. I'll try to fix things and get back to you.
Edit: sorry. I missed something.
c515492
to
9c764ce
Compare
76db12c
to
0ed959b
Compare
0ed959b
to
118084c
Compare
docs/pages/container.rst
Outdated
>>> from returns.result import Success | ||
>>> | ||
>>> value = Success(1) | ||
>>> # We can use map to effectively replace the inner value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, that's not correct :(
This way we can only replace values in "truthy" values. It will not replace a value in Failure(1)
, for example.
118084c
to
2fa9fda
Compare
@sobolevn , sorry about the delay. My family got sick and then I got sick and then work has been busy since I missed some time. I went through this again and resolved what I think is what you want. Please review again please. |
828b34c
to
3b9768d
Compare
69cab3c
to
c68ea9f
Compare
…) support - Added the __replace__ magic method to BaseContainer, enabling the creation of modified copies of immutable containers in line with Python 3.13's copy.replace() functionality. - Updated documentation to include usage examples and clarify the behavior of the new method. - Added tests to ensure the correct functionality of the __replace__ method and its integration with the copy module. - Updated CHANGELOG to reflect this new feature and its implications for container usage. Closes dry-python#1920.
c68ea9f
to
51936c3
Compare
@sobolevn , this is ready for your review. Thanks so much! |
I have made things!
Checklist
CHANGELOG.md
Related issues
copy.replace
from 3.13 #1920🙏 Please, if you or your company finds
dry-python
valuable, help us sustain the project by sponsoring it transparently on https://github.com/sponsors/dry-python. As a thank you, your profile/company logo will be added to our main README which receives hundreds of unique visitors per day.