Repository navigation
gh-160106: Fix csv.Dialect.__replace__() losing slot-backed attributes - #160641
saiyam-jain-dev wants to merge 3 commits into
Conversation
|
All commit authors signed the Contributor License Agreement. |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
…ributes copy.replace() on a csv.Dialect subclass that stores dialect options in __slots__ would silently drop those attributes. __replace__() only copied self.__dict__, so any attribute stored in a slot was missing from the new object, causing csv.reader/writer to fall back to the default value. Fix by iterating the MRO for __slots__ entries and copying any dialect attribute that was not already set via __dict__.
68b5d3d to
181df51
Compare
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
sobolevn
left a comment
sobolevn
left a comment
There was a problem hiding this comment.
please, test this properly. provide multiple case with different __slots__ setups.
| new.__dict__.update(self.__dict__) | ||
| # Copy slot-backed dialect attributes from the instance's MRO, | ||
| # since __dict__ only captures attributes stored in __dict__. | ||
| for cls in type(self).__mro__: |
There was a problem hiding this comment.
One more thing: it looks like a rather popular task, including user-facing objects. Maybe we should provide a generic helper for this? You can open a new issue to discuss this
There was a problem hiding this comment.
Good idea, having a standard helper for copying/replacing slotted attributes would be really useful across the stdlib and user code. I'll open an issue for it so we can discuss the design.
|
Added tests in est_csv covering multiple slot setups:
Also updated the implementation to use setattr(): slot descriptors shadow dict, so writing directly to dict left the slots uninitialized on the new instance and readers still fell back to defaults. It now sets the descriptors directly and handles slotted overrides cleanly. |
Summary
Fixes #160106
copy.replace() on a csv.Dialect subclass that stores dialect options in slots silently drops those slot-backed attributes. Dialect.replace() only copied self.dict, so any attribute stored in a slot (rather than dict) was missing from the new object, causing csv.reader/csv.writer to fall back to the default comma delimiter.
Reproducer (before fix)
`python
import copy, csv
class SemicolonDialect(csv.excel):
slots = ('delimiter',)
def init(self):
self.delimiter = ';'
super().init()
dialect = SemicolonDialect()
replacement = copy.replace(dialect, quoting=csv.QUOTE_ALL)
print(list(csv.reader(['alpha;beta'], dialect))) # [['alpha', 'beta']]
print(list(csv.reader(['alpha;beta'], replacement))) # [['alpha;beta']] ← BUG
`
Fix
In Dialect.replace(), after copying self.dict, iterate the MRO for slots entries and copy any dialect attribute not already set via dict before applying the caller's changes.
Testing
The existing csv test suite continues to pass. The reproducer from the issue now produces [['alpha', 'beta']] for both readers.