Fix copying Message in pure-python. #15
Conversation
Do it like the C extension does, by actually copying from the other object and then setting attributes if needed. Also drop old C support code for Python < 2.6
src/zope/i18nmessageid/message.py
Outdated
| self.default = ustr.default[:] if ustr.default else None | ||
| self.mapping = ustr.mapping.copy() if ustr.mapping is not None else None | ||
| self.msgid_plural = ustr.msgid_plural[:] if ustr.msgid_plural else None | ||
| self.default_plural = ustr.default_plural[:] if ustr.default_plural else None |
There was a problem hiding this comment.
Why are we even trying to slice strings anyway? a_string[:] is a_string in CPython AFAICT. Is the C code doing slices?
(Also I'm mildly unhappy about explicitly making a copy of the mapping dictionary and then throwing it away. I suppose it's a rare occurrence, but still.)
There was a problem hiding this comment.
I wondered that too. The C code is not slicing strings. Nor does it copy() the mapping. Suppose we just remove that here?
There was a problem hiding this comment.
No tests broke if that stops happening. I removed it and added tests to confirm that Python and C are handling the objects the same way.
…trings and (somewhat pointedly) copy mappings, like the C implementation.
|
Thanks! Let's roll the dice. Can we get a PyPI release, please? (Or access for |
|
I started the release process and uploaded the git tags before I realised I didn't have PyPI rights. So the 4.3.1 wheels are showing up on PyPI now thanks to the CI services, but not a sdist (.tar.gz). |
|
I just added you as an owner of the PyPI project. |
|
Thank you! I've uploaded the corresponding .tar.gz |
|
For packages where CI uploads wheels perhaps we should also upload sdists (using |
Do it like the C extension does, by actually copying from the other object and then setting attributes if needed.
Also drop old C support code for Python < 2.6
Fixes #14