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

build: update dependencies #1720

Merged
merged 2 commits into from
Nov 4, 2024
Merged

Conversation

havogt
Copy link
Contributor

@havogt havogt commented Nov 1, 2024

updates minimum gridtools_cpp version for #1699

@havogt havogt mentioned this pull request Nov 1, 2024
@@ -106,11 +106,11 @@ def get_location(self, node: ast.AST) -> SourceLocation:

# `FixMissingLocations` ensures that all nodes have the location attributes
assert hasattr(node, "lineno")
line = node.lineno + line_offset if node.lineno is not None else None
line = node.lineno + line_offset
Copy link
Contributor Author

Choose a reason for hiding this comment

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

let's see if that works (mypy complain because line, colum are not Optional)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah great they fixed this in python.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, you misinterpreted. eve.SourceLocation had this always as non-optional and I trust FixMissingLocations to respect that. Nothing changed in Python.

@havogt havogt requested a review from egparedes November 1, 2024 16:32
@@ -106,11 +106,11 @@ def get_location(self, node: ast.AST) -> SourceLocation:

# `FixMissingLocations` ensures that all nodes have the location attributes
assert hasattr(node, "lineno")
line = node.lineno + line_offset if node.lineno is not None else None
line = node.lineno + line_offset
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah great they fixed this in python.

@@ -140,7 +140,7 @@ def impl(*args: Any | tuple[Any | tuple, ...]) -> _R | tuple[_R | tuple, ...]:
assert result_collection_constructor is not None
return result_collection_constructor(impl(*arg) for arg in zip(*args))

return fun( # type: ignore[misc] # mypy not smart enough
return fun( # type: ignore[call-arg, misc] # mypy not smart enough
Copy link
Contributor

Choose a reason for hiding this comment

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

The cast is probably superfluous together with call-arg, but whatever.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I double checked. The cast actually fixes a arg-type error. The call-arg is for "too few arguments", not motivated to investigate further.

@havogt havogt merged commit 162a512 into GridTools:main Nov 4, 2024
55 checks passed
@havogt havogt deleted the update_dependencies branch November 4, 2024 07:53
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