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

Update dependencies to match those in 'functional' #978

Merged
merged 3 commits into from
Oct 20, 2022
Merged

Update dependencies to match those in 'functional' #978

merged 3 commits into from
Oct 20, 2022

Conversation

jdahm
Copy link
Contributor

@jdahm jdahm commented Oct 14, 2022

Description

... and fix the few errors that the new mypy version catches.

@jdahm jdahm requested a review from havogt October 14, 2022 17:37
Comment on lines 188 to 190
if self.path_stats(self.path) != self.path_stats(str(self.module_file.absolute())):
assert fullname is not None
self.module_file.write_text(self.get_source_code(fullname))
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 wasn't quite sure how to handle this one, or how this function is used.

Comment on lines 134 to +135
- repo: https://github.com/pre-commit/mirrors-mypy
rev: v0.812
rev: v0.982
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another option here is to use the one installed with requirements-dev.txt.

Copy link
Contributor

@egparedes egparedes left a comment

Choose a reason for hiding this comment

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

The typing-extensions requirements needs to be changed but otherwise looks good to me.

setup.cfg Outdated Show resolved Hide resolved
Co-authored-by: Enrique G. Paredes <18477+egparedes@users.noreply.github.com>
@jdahm
Copy link
Contributor Author

jdahm commented Oct 19, 2022

bors try

bors bot added a commit that referenced this pull request Oct 19, 2022
@bors
Copy link

bors bot commented Oct 19, 2022

try

Build failed:

@jdahm
Copy link
Contributor Author

jdahm commented Oct 19, 2022

bors try

Copy link
Contributor

@egparedes egparedes left a comment

Choose a reason for hiding this comment

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

It looks good to me.

@egparedes
Copy link
Contributor

bors try

@bors
Copy link

bors bot commented Oct 20, 2022

try

Already running a review

@jdahm
Copy link
Contributor Author

jdahm commented Oct 20, 2022

bors try-
bors try

bors bot added a commit that referenced this pull request Oct 20, 2022
@bors
Copy link

bors bot commented Oct 20, 2022

try

Build succeeded:

@jdahm jdahm merged commit 2912d8c into GridTools:master Oct 20, 2022
@jdahm jdahm deleted the update-deps-to-functional branch October 20, 2022 19:07
jdahm added a commit that referenced this pull request Oct 26, 2022
Major changes:

- Adds type checking on a few more files.
- Expr.dtype's type is no longer Optional[DataType], because that caused type havoc. Therefore, the default is changed from None to DataType.AUTO when not set explicitly. The gtir_dtype_resolver and other visitors are updated to check for DataType.AUTO instead of None.
- Moves most of the code that was in eve.iterators to eve.concepts to work around a circular dependency after solving ClassVar parameters cannot include any type variables #565 using @egparedes's suggestion. This is a temporary workaround until we replace Eve with the new version.
- gt4py.stencil_object.ArgsInfo is made non-private and used in gt4py.dace_stencil_object to fix a type issue.
- Ignores typing in eve visitor contexts -- need to fix that in a follow-up PR.
- npir.HorizontalMask is created as a tuple of tuples, to avoid the Optional[AxisBound] in the common.HorizontalInterval type.
- Replaces gt_backend references with gt4py.backend.

Follows #978.

Resolves #565.
Resolves #992.
Resolves #1007.
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.

None yet

2 participants