Stop copying parent attributes into child class #330
Conversation
|
This is a highly speculative PR to see what fails on CI when this code is removed. |
This avoids breaking the normal MRO for attributes, which searches the class hierarchy for attribute definitions. If attributes are copied onto every class in the hierarchy, the C3 MRO algorithm finds the attribute in a child class even though that class didn't actually define it, which results in surprising attribute values when classes in the hierarchy set the same attribute to different values.
|
@jezdez, could the workflow get an approval please? |
Codecov Report
@@ Coverage Diff @@
## master #330 +/- ##
==========================================
- Coverage 89.98% 89.94% -0.04%
==========================================
Files 25 25
Lines 1198 1194 -4
Branches 216 213 -3
==========================================
- Hits 1078 1074 -4
Misses 89 89
Partials 31 31
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Seems to have passed the test suite (the failing tests seem to be caused by something else). |
|
I think we'd want to tweak this so that the deprecated settings check still looks at inheriting settings, but otherwise it appears that this won't break any functionality. However, it will change the MRO for settings, so users of django-configurations could have setting values change unexpectedly. If we decided to go ahead with this fix, then I think it'd call for a major version bump. @jezdez, if that's something you'd be happy with, then I'll tweak this PR so that it's ready for review. |
The change seems reasonable to me, can you work on this PR to update it, adding deprecation notes and tests? |
This avoids breaking the normal MRO for attributes, which searches the class hierarchy for attribute definitions. If attributes are copied onto every class in the hierarchy, the C3 MRO algorithm finds the attribute in a child class even though that class didn't actually define it, which results in surprising attribute values when classes in the hierarchy set the same attribute to different values.
Fixes #329