refactor self.selections per Qt's pattern #206
Conversation
prjemian
commented
Feb 28, 2024
prjemian
commented
- close self.selections should follow Qt's get/set pattern #201
|
@MDecarabas left that one to me, @prjemian I'll get to it later today. |
gemviz/select_fields_tablemodel.py
Outdated
| def selections(self, key): | ||
| """Pick the key from the plot selections dictionary.""" | ||
| return self._selections[key] |
There was a problem hiding this comment.
As far as I can tell this function is not used anywhere in the code, except for line 222. Even when used it does not seem to be any shorter or cleaner than the previous implementation. Is there a specific problem you are trying to address?
There was a problem hiding this comment.
I tend to agree. It's a question of consistency (with Qt's pattern) or being concise. Not sure these changes help.
@rodolakis - What do you think?
There was a problem hiding this comment.
I am looking to see if I am using this at some other places in the code in mdaviz (stuff that would come up afterward as the development progresses)
There was a problem hiding this comment.
The def selections(self) method is still not consistent with Qt pattern: self._selections attribute is a dictionary, so self.selections() should also return a dictionary.
gemviz/select_fields_tablemodel.py
Outdated
|
|
||
| def setSelectionsItem(self, key, value): | ||
| """Set the key in the plot selections dictionary.""" | ||
| self.selections()[key] = value |
There was a problem hiding this comment.
We should not modify the method (self.selections) but the attribute (self._selections)
