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

chore: Remove Mypy errors from examples. #2171

Open
wants to merge 28 commits into
base: main
Choose a base branch
from
Open

Conversation

7460m
Copy link

@7460m 7460m commented Oct 22, 2023

The PR fulfills these requirements: (check all the apply)

Fixed: #2047

  • It's submitted to the main branch.
  • When resolving a specific issue, it's referenced in the PR's title (e.g. feat: Add a button #xxx, where "xxx" is the issue number).
  • When resolving a specific issue, the PR description includes Closes #xxx, where "xxx" is the issue number.
  • If changes were made to ui folder, unit tests (make test) still pass.
  • New/updated tests are included

Mypy result before:
Fixed:Found 218 errors in 23 files (checked 273 source files)

Mypy result after:
Success: no issues found in 273 source files

@mturoci
Copy link
Collaborator

mturoci commented Oct 24, 2023

Thanks for the PR @7460m! Did you manually verify all the examples work as before and their behavior was not changed after your refactors?

Copy link
Collaborator

@mturoci mturoci left a comment

Choose a reason for hiding this comment

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

Thanks @7460m! A few comments from me.

py/examples/graphics_spline.py Outdated Show resolved Hide resolved
py/examples/db_todo.py Outdated Show resolved Hide resolved
py/examples/db_todo.py Outdated Show resolved Hide resolved
Comment on lines 21 to 23
g.spline(x=[float(v) for v in x], y=[float(v) for v in y], curve='linear', style=line_style),
g.spline(x=[float(v) for v in x], y=[float(v) for v in y], curve='basis', style=line_style),
g.spline(x=[float(v) for v in x], y=[float(v) for v in y], curve='basis-closed', style=line_style),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This makes code harder to follow. Is there any other way to make mypy happy while preserving the original way of passing params to spline?

Copy link
Author

Choose a reason for hiding this comment

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

Updated with a function to make it clear, I've try to modify x outside of the list and it seems not working

Copy link
Author

Choose a reason for hiding this comment

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

also I've manually checked before and after, the line seems thinner because of the **line_style -> style=line_style. let me know whats you thoughts on fixing this

Comment on lines 29 to 33
c=[(0.90687198, 0.0, 0.0), (0.78837333, 0.0, 0.0), (0.76840584, 0.0, 0.0), (0.59849648, 0.0, 0.0), (0.44214562, 0.0, 0.0), (0.72303802, 0.0, 0.0),
(0.41661825, 0.0, 0.0), (0.2268104, 0.0, 0.0), (0.45422734, 0.0, 0.0), (0.84794375, 0.0, 0.0), (0.93665595, 0.0, 0.0), (0.95603618, 0.0, 0.0),
(0.39209432, 0.0, 0.0), (0.70832467, 0.0, 0.0), (0.12951583, 0.0, 0.0), (0.35379639, 0.0, 0.0), (0.40427152, 0.0, 0.0), (0.6485339, 0.0, 0.0),
(0.03307097, 0.0, 0.0), (0.53800936, 0.0, 0.0), (0.13171312, 0.0, 0.0), (0.52093493, 0.0, 0.0), (0.10248479, 0.0, 0.0), (0.15798038, 0.0, 0.0),
(0.92002965, 0.0, 0.0)],
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is this for?

Copy link
Author

Choose a reason for hiding this comment

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

image.py:29: error: List item 1 has incompatible type "float"; expected "tuple[float, float, float] | str | tuple[float, float, float, float] | tuple[tuple[float, float, float] | str, float] | tuple[tuple[float, float, float, float], float]" [list-item]

Copy link
Author

Choose a reason for hiding this comment

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

I'll work on it to see if there's another way, its currently all red dots..

Copy link
Author

Choose a reason for hiding this comment

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

I've looked into this one, in the plt.scatter document said "A scalar or sequence of n numbers to be mapped to colors using cmap and norm." which I think the original code is perfectly fine, but mypy doesn't think so, ill put ignore on it for now, if you want me to put in random value to satisfy the mypy let me know.

py/examples/table_markdown_pandas.py Show resolved Hide resolved
py/examples/tour.py Outdated Show resolved Hide resolved
py/examples/table_pagination.py Outdated Show resolved Hide resolved
Comment on lines +31 to +32
str_cols = df.select_dtypes(include=['object']).columns
return df[df[str_cols].apply(lambda column: column.str.contains(term, case=False, na=False)).any(axis=1)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

What mypy err does this resolve?

Copy link
Author

Choose a reason for hiding this comment

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

table_filter_backend.py:31: error: List item 0 has incompatible type "type[object]"; expected "str" [list-item]

Copy link
Author

Choose a reason for hiding this comment

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

The visual effect looks the same

@mturoci mturoci changed the title Issue/#2047 chore: Remove Mypy errors from examples. Oct 24, 2023
@7460m
Copy link
Author

7460m commented Oct 24, 2023

Hi mturoci,

Thanks for the review, I"ll work on it and get back to you asap

@7460m 7460m requested a review from mturoci October 28, 2023 01:13
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.

Remove mypy errors from python examples
2 participants