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

Fix ruff re errors #289

Merged
merged 3 commits into from
Nov 25, 2024
Merged

Fix ruff re errors #289

merged 3 commits into from
Nov 25, 2024

Conversation

alexfikl
Copy link
Collaborator

No description provided.

@@ -190,7 +190,7 @@ def map_data_wrapper(self, expr: DataWrapper) -> Array:

# https://github.com/pylint-dev/pylint/issues/3893
# pylint: disable=unexpected-keyword-arg
return DataWrapper(
return DataWrapper( # type: ignore[call-arg]
Copy link
Owner

Choose a reason for hiding this comment

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

I actually don't understand what mypy is complaining about here. shape and data are defined as dataclass attributes?

Copy link
Collaborator Author

@alexfikl alexfikl Nov 24, 2024

Choose a reason for hiding this comment

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

Yeah, I really don't know either :\ I've stared a bit at the pytato code, but it's obviously fine. It's doubly surprising since those two attributes are actually in the class itself, not in a parent..

Definitely the same thing that's confusing pylint as well though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For what it's worth, doing a reveal_type(DataWrapper) says

arraycontext/impl/pytato/utils.py:193: note: Revealed type is "def (*, axes: builtins.tuple[pytato.array.Axis, ...], tags: builtins.frozenset[pytools.tag.Tag], non_equality_tags: builtins.frozenset[pytools.tag.Tag] =) -> pytato.array.DataWrapper"

so for some reason it really doesn't think those arguments are there..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Possibly related to this: python/mypy#17375

Copy link
Owner

Choose a reason for hiding this comment

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

Good find, thanks. Added that as context in a comment so we don't lose it.

@inducer inducer enabled auto-merge (rebase) November 25, 2024 03:24
@inducer inducer merged commit 1ec60bf into inducer:main Nov 25, 2024
12 checks passed
@alexfikl alexfikl deleted the fix-ruff branch November 25, 2024 06:17
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