Type check ert.gui #8160
Type check ert.gui #8160
Conversation
eivindjahren
force-pushed
the
type_check_ertwidgets
branch
2 times, most recently
from
c58acf6 to
faeebf8
Compare
eivindjahren
force-pushed
the
type_check_ertwidgets
branch
11 times, most recently
from
84ac680 to
3ce383f
Compare
eivindjahren
added
the
release-notes:skip
eivindjahren
force-pushed
the
type_check_ertwidgets
branch
2 times, most recently
from
2c764b7 to
44ff307
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8160 +/- ##
==========================================
- Coverage 87.18% 87.11% -0.07%
==========================================
Files 381 381
Lines 23669 23757 +88
Branches 619 630 +11
==========================================
+ Hits 20635 20696 +61
- Misses 2953 2988 +35
+ Partials 81 73 -8 ☔ View full report in Codecov by Sentry. |
| def setText(self, a0: Optional[str]) -> None: | ||
| self.hidePlaceHolder() | ||
|
|
||
| QLineEdit.setText(self, string) | ||
| QLineEdit.setText(self, a0) | ||
|
|
||
| if len(str(string)) == 0 and not self.hasFocus(): | ||
| if len(str(a0)) == 0 and not self.hasFocus(): |
There was a problem hiding this comment.
Should this have been 'text' instead, since this is not an event?
text : Optional[str]
There was a problem hiding this comment.
The setText function in QLineEdit names the first parameter a0 and sets the type to Optional[str], so that is what we have to use.
| if (viewport := self.viewport()) is not None: | ||
| viewport.setMouseTracking(True) |
There was a problem hiding this comment.
Why the walrus operator here? We don't need viewport later on?
if self.viewport():
self.viewport().setMouseTracking(True)
There was a problem hiding this comment.
Because two calls to self.viewport() could first return None and then something else. We kind of know this is a getter, but mypy doesn't care.
| if self._running_workflow_dialog is not None: | ||
| self._running_workflow_dialog.disableCloseButton() |
There was a problem hiding this comment.
More compact?
if self._running_workflow_dialog:
self._running_workflow_dialog.disableCloseButton()
There was a problem hiding this comment.
Maybe, but I think its best to avoid messing with __bool__ for now.
|
|
||
| def runWorkflow(self) -> None: | ||
| assert self._workflow_runner is not None |
There was a problem hiding this comment.
Why not assert it simply? Is that not the same?
assert self._workflow_runner
There was a problem hiding this comment.
Just in case __bool__ is overriden somewhere, I would prefer to keep it the way it is.
| horizontal_header = self.horizontalHeader() | ||
| assert horizontal_header is not None | ||
| horizontal_header.setSectionResizeMode(QHeaderView.ResizeToContents) |
There was a problem hiding this comment.
why save this as a variable?
There was a problem hiding this comment.
So that we can assert that it is None. Although self.horizontlHeader() is intented to be a property, it is called as a function so two subsequent calls could first return the horizontal header and next return None.
eivindjahren
force-pushed
the
type_check_ertwidgets
branch
3 times, most recently
from
a6d81f7 to
093c501
Compare
eivindjahren
force-pushed
the
type_check_ertwidgets
branch
from
093c501 to
a70084c
Compare
eivindjahren
force-pushed
the
type_check_ertwidgets
branch
from
a70084c to
e4bcfbb
Compare
When applicable