Also remove super arguments in rst files#6629
Conversation
|
Hi there @MSeifert04 👋 - thanks for the pull request! I'm just a friendly 🤖 that checks for issues related to the changelog and making sure that this pull request is milestoned and labelled correctly. This is mainly intended for the maintainers, so if you are not a maintainer you can ignore this, and a maintainer will let you know if any action is required on your part 😃. Everything looks good from my point of view! 👍 If there are any issues with this message, please report them here |
| ... # Call the setter of the super class in case it might contain some | ||
| ... # important logic (only True for meta, unit and uncertainty) | ||
| ... super(NDDataUncertaintyShapeChecker, self.__class__).uncertainty.fset(self, value) | ||
| ... NDData.uncertainty.fset(self, value) |
There was a problem hiding this comment.
Note that you can't easily use super when just overriding the setter of a property. This is probably better than using self.__class__ as second argument... At least it's shorter and doesn't look like a bug waiting to happen.
There was a problem hiding this comment.
If @mwcraig approves this, I don't see a problem with the rest. Thanks!
There was a problem hiding this comment.
Yeah, I'm wondering if I shouldn't revert that one again. Hardcoding the superclass also has it's drawbacks because it breaks the MRO...
But on the other hand it already uses @NDData.uncertainty.setter as decorator so it will probably (untested) break the MRO anyway...
There was a problem hiding this comment.
This indeed feels very hacky. My tendency would be to leave it in place for this PR (which looks all OK apart from this), and open a separate issue about it (unless @mwcraig has an immediate answer).
There was a problem hiding this comment.
I don't have a better solution...except perhaps removing the example 😉. (I know, not really an option) Opening a separate issue in a moment.
| ... # Call the setter of the super class in case it might contain some | ||
| ... # important logic (only True for meta, unit and uncertainty) | ||
| ... super(NDDataUncertaintyShapeChecker, self.__class__).uncertainty.fset(self, value) | ||
| ... NDData.uncertainty.fset(self, value) |
There was a problem hiding this comment.
I don't have a better solution...except perhaps removing the example 😉. (I know, not really an option) Opening a separate issue in a moment.
|
I reverted the problematic change. Better to let it as-is than to provide a similar messy way. |
|
Looks all OK now, so merging. Thanks, @MSeifert04! |
|
@MSeifert04 , I think you have unlocked the "super cleaner" achievement. 🏅 |
Like #6613 but I forgot to change the
.rstfiles there...