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

Activate Pylint Rules #976

Merged
merged 9 commits into from
Jan 16, 2024
Merged

Conversation

zariiii9003
Copy link
Contributor

Can you check these?

  style: commands[1]> ruff check ./src
  src/asammdf/gui/widgets/plot.py:136:8: PLR0124 Name compared with itself, consider replacing `b != b`
  src/asammdf/gui/widgets/plot.py:144:8: PLR0124 Name compared with itself, consider replacing `b != b`
  src/asammdf/gui/widgets/plot.py:4460:28: PLR0124 Name compared with itself, consider replacing `min_ != min_`
  src/asammdf/gui/widgets/plot.py:4462:28: PLR0124 Name compared with itself, consider replacing `max_ != max_`
  src/asammdf/gui/widgets/plot.py:4503:28: PLR0124 Name compared with itself, consider replacing `min_ != min_`
  src/asammdf/gui/widgets/plot.py:4505:28: PLR0124 Name compared with itself, consider replacing `max_ != max_`
  src/asammdf/gui/widgets/tabular_base.py:1382:16: PLR0124 Name compared with itself, consider replacing `target != target`
  Found 7 errors.



if min_ != min_:

if max_ != max_:

if min_ != min_:

if max_ != max_:

if target != target:

@danielhrisca
Copy link
Owner

Those comparison are to check is the value is not a number NaN

@zariiii9003
Copy link
Contributor Author

Those comparison are to check is the value is not a number NaN

Ah, i didn't know that. Would you mind if i replace it with np.isnan for readability?

@danielhrisca
Copy link
Owner

My concern is the execution speed in this case

image

Copy link
Owner

@danielhrisca danielhrisca left a comment

Choose a reason for hiding this comment

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

please address the open topics

@@ -6593,7 +6590,7 @@ def get(
if name is None:
name = channel.name

unit = conversion and conversion.unit or channel.unit
unit = conversion.unit if conversion else channel.unit
Copy link
Owner

Choose a reason for hiding this comment

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

This is not right: if the conversion unit is empty then we need to check the channel unit as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True. I just found out, that the preview rule, which flagged this, will be removed soon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -174,7 +174,7 @@ def search_text_changed(self):
(group_index, channel_index): {
"names": [ch.name],
"comment": extract_xml_comment(ch.comment).strip(),
"unit": ch.conversion and ch.conversion.unit or ch.unit,
"unit": ch.conversion.unit if ch.conversion else ch.unit,
Copy link
Owner

Choose a reason for hiding this comment

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

same as above, also applies to the following commit diffs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

src/asammdf/gui/widgets/tabular_base.py Show resolved Hide resolved
@danielhrisca danielhrisca merged commit cbef707 into danielhrisca:development Jan 16, 2024
18 checks passed
@danielhrisca
Copy link
Owner

thank you!

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