Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change "is not" to "!=" when checking literals #886

Open
wants to merge 1 commit into
base: 1.2.1-release
Choose a base branch
from

Conversation

KianKhadempour
Copy link

In a most widget files is not is used to check the error against an empty string (''). This doesn't work all the time to my knowledge, so it is best to replace it with "!=", which is an equality check.

Example

Before:

def syncUiState(self, state: FormField):  # type: ignore
        self.widget.setValue(state['value'])  # type: ignore
        self.error.SetLabel(state['error'] or '')
        self.error.Show(state['error'] is not None and state['error'] is not '')

After:

def syncUiState(self, state: FormField):  # type: ignore
        self.widget.setValue(state['value'])  # type: ignore
        self.error.SetLabel(state['error'] or '')
        self.error.Show(state['error'] is not None and state['error'] != '')
#       --------------------------------------------------------------^^----

Checklist:

  • You're opening this PR against the current release branch
  • Works on both Python 2.7 & Python 3.x
  • Commits have been squashed and includes the relevant issue number

Squashing is not necessary (only one commit) and there is no relevant issue.

  • Pull request description contains link to relevant issue or detailed notes on changes
  • This must include example code demonstrating your feature or the bug being fixed
  • All existing tests pass
  • Your bug fix / feature has associated test coverage

N/A

  • README.md is updated (if relevant)

N/A

@@ -183,7 +183,7 @@ def getUiState(self) -> t.FormField:
def syncUiState(self, state: FormField): # type: ignore
self.widget.setValue(state['value']) # type: ignore
self.error.SetLabel(state['error'] or '')
self.error.Show(state['error'] is not None and state['error'] is not '')
self.error.Show(state['error'] is not None and state['error'] != '')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't it enough to write

self.error.Show(bool(state['error']))

?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems to work as well, assuming we want other falsy values to equate to false as well (such as 0).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants