Skip to content

Internal error checking xarray with --check-untyped-defs #11007

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

Closed
tlambert03 opened this issue Aug 21, 2021 · 7 comments · Fixed by #11061
Closed

Internal error checking xarray with --check-untyped-defs #11007

tlambert03 opened this issue Aug 21, 2021 · 7 comments · Fixed by #11061
Labels

Comments

@tlambert03
Copy link

Crash Report

Not sure whether this is a mypy issue, or an xarray issue... but I'm getting RuntimeError: TypeGuard should not appear here when running mypy on a file that imports xarray when using the --check-untyped-defs flag. Minimal env on mypy master:

To Reproduce

conda create -n test -c conda-forge python
conda activate test
pip install xarray
pip install git+https://github.com/python/mypy.git
echo "import xarray" > m.py
mypy --check-untyped-defs --show-traceback m.py

Traceback

❯ mypy --check-untyped-defs --show-traceback m.py
/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/xarray/core/common.py:1659: error: INTERNAL ERROR -- Please try using mypy master on Github:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 0.920+dev.7576f659d42ee271c78e69d7481b9d49517a49f6
Traceback (most recent call last):
  File "/Users/talley/miniconda3/envs/test/bin/mypy", line 8, in <module>
    sys.exit(console_entry())
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/__main__.py", line 11, in console_entry
    main(None, sys.stdout, sys.stderr)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/main.py", line 87, in main
    res, messages, blockers = run_build(sources, options, fscache, t0, stdout, stderr)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/main.py", line 165, in run_build
    res = build.build(sources, options, None, flush_errors, fscache, stdout, stderr)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/build.py", line 179, in build
    result = _build(
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/build.py", line 254, in _build
    graph = dispatch(sources, manager, stdout)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/build.py", line 2707, in dispatch
    process_graph(graph, manager)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/build.py", line 3031, in process_graph
    process_stale_scc(graph, scc, manager)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/build.py", line 3129, in process_stale_scc
    graph[id].type_check_first_pass()
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/build.py", line 2175, in type_check_first_pass
    self.type_checker().check_first_pass()
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checker.py", line 295, in check_first_pass
    self.accept(d)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checker.py", line 402, in accept
    stmt.accept(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/nodes.py", line 530, in accept
    return visitor.visit_overloaded_func_def(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checker.py", line 435, in visit_overloaded_func_def
    self._visit_overloaded_func_def(defn)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checker.py", line 457, in _visit_overloaded_func_def
    defn.impl.accept(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/nodes.py", line 696, in accept
    return visitor.visit_func_def(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checker.py", line 734, in visit_func_def
    self._visit_func_def(defn)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checker.py", line 738, in _visit_func_def
    self.check_func_item(defn, name=defn.name)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checker.py", line 800, in check_func_item
    self.check_func_def(defn, typ, name)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checker.py", line 983, in check_func_def
    self.accept(item.body)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checker.py", line 402, in accept
    stmt.accept(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/nodes.py", line 1024, in accept
    return visitor.visit_block(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checker.py", line 1996, in visit_block
    self.accept(s)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checker.py", line 402, in accept
    stmt.accept(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/nodes.py", line 1216, in accept
    return visitor.visit_if_stmt(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checker.py", line 3313, in visit_if_stmt
    self.accept(b)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checker.py", line 402, in accept
    stmt.accept(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/nodes.py", line 1024, in accept
    return visitor.visit_block(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checker.py", line 1996, in visit_block
    self.accept(s)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checker.py", line 402, in accept
    stmt.accept(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/nodes.py", line 1216, in accept
    return visitor.visit_if_stmt(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checker.py", line 3303, in visit_if_stmt
    t = get_proper_type(self.expr_checker.accept(e))
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checkexpr.py", line 3910, in accept
    typ = node.accept(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/nodes.py", line 1663, in accept
    return visitor.visit_unary_expr(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checkexpr.py", line 2873, in visit_unary_expr
    operand_type = self.accept(e.expr)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checkexpr.py", line 3910, in accept
    typ = node.accept(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/nodes.py", line 1600, in accept
    return visitor.visit_call_expr(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checkexpr.py", line 277, in visit_call_expr
    return self.visit_call_expr_inner(e, allow_none_return=allow_none_return)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checkexpr.py", line 366, in visit_call_expr_inner
    ret_type = self.check_call_expr_with_callee_type(callee_type, e, fullname,
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checkexpr.py", line 873, in check_call_expr_with_callee_type
    return self.check_call(callee_type, e.args, e.arg_kinds, e,
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checkexpr.py", line 933, in check_call
    return self.check_callable_call(callee, args, arg_kinds, context, arg_names,
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checkexpr.py", line 1030, in check_callable_call
    self.check_argument_types(arg_types, arg_kinds, args, callee, formal_to_actual, context,
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checkexpr.py", line 1493, in check_argument_types
    check_arg(expanded_actual, actual_type, arg_kinds[actual],
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/checkexpr.py", line 1523, in check_arg
    elif not is_subtype(caller_type, callee_type):
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/subtypes.py", line 93, in is_subtype
    return _is_subtype(left, right,
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/subtypes.py", line 147, in _is_subtype
    return left.accept(SubtypeVisitor(orig_right,
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/types.py", line 1747, in accept
    return visitor.visit_union_type(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/subtypes.py", line 476, in visit_union_type
    return all(self._is_subtype(item, self.orig_right) for item in left.items)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/subtypes.py", line 476, in <genexpr>
    return all(self._is_subtype(item, self.orig_right) for item in left.items)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/subtypes.py", line 205, in _is_subtype
    return is_subtype(left, right,
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/subtypes.py", line 93, in is_subtype
    return _is_subtype(left, right,
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/subtypes.py", line 147, in _is_subtype
    return left.accept(SubtypeVisitor(orig_right,
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/types.py", line 296, in accept
    return visitor.visit_type_guard_type(self)
  File "/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/mypy/subtypes.py", line 479, in visit_type_guard_type
    raise RuntimeError("TypeGuard should not appear here")
RuntimeError: TypeGuard should not appear here
/Users/talley/miniconda3/envs/test/lib/python3.9/site-packages/xarray/core/common.py:1659: : note: use --pdb to drop into pdb

Your Environment

  • Mypy version used: 0.920+dev.7576f659d42ee271c78e69d7481b9d49517a49f6
  • Mypy command-line flags: --check-untyped-defs
  • Mypy configuration options from mypy.ini (and other config files): no config files
  • Python version used: 3.9.6
  • Operating system and version: macos 10.15.7
❯ pip list
Package           Version
----------------- --------------------------------------------------
mypy              0.920+dev.7576f659d42ee271c78e69d7481b9d49517a49f6
mypy-extensions   0.4.3
numpy             1.21.2
pandas            1.3.2
pip               21.2.4
python-dateutil   2.8.2
pytz              2021.1
setuptools        57.4.0
six               1.16.0
tomli             1.1.0
typing-extensions 3.10.0.0
wheel             0.37.0
xarray            0.19.0
@JelleZijlstra
Copy link
Member

This is surely a bug in mypy, thanks for the report!

I don't know what version of xarray you're running exactly, but looking at master right now, there is a call to is_scalar at line 1646 (https://github.com/pydata/xarray/blob/main/xarray/core/common.py#L1646). Mypy's TypeGuard support is a little buggy in general, so we probably just need to add a case somewhere.

@hauntsaninja
Copy link
Collaborator

Probably the same as #10647

Also not the first time we've had an issue stemming from xarray's typing (#9437), could be good to ensure mypy_primer coverage...

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Aug 23, 2021

( @tlambert03 if your project using xarray + mypy is open source, let me know, and I can add it to mypy_primer. It looks like I already have xarray in there, but clearly that wasn't enough)

edit: found https://github.com/arviz-devs/arviz.git which reproduces the crash

@JelleZijlstra
Copy link
Member

The repro case here apparently is just importing xarray, so it's not clear why mypy-primer wouldn't catch that.

@hauntsaninja
Copy link
Collaborator

My guess would be because xarray's mypy.ini doesn't have check untyped defs

@tlambert03
Copy link
Author

tlambert03 commented Aug 23, 2021

here's the most relevant thing I could find about the way xarray uses mypy internally:

https://github.com/pydata/xarray/blob/808b2e9eec1ec48d43cf9311dee27cd5550b8fd2/setup.cfg#L242-L245

[mypy]
exclude = properties|asv_bench|doc
files = xarray/**/*.py
show_error_codes = True

# version spanning code is hard to type annotate (and most of this module will
# be going away soon anyways)
[mypy-xarray.core.pycompat]
ignore_errors = True

@tlambert03
Copy link
Author

but I'm not an xarray developer, I work on a project (napari) which had an xarray import which broke my ability to test our own package. The minimal reproducible example above is just a fresh environment with a couple installs, a single import xarray statement with --check-untyped-defs

hauntsaninja added a commit that referenced this issue Sep 14, 2021
Fixes #11007, fixes #10899, fixes #10647

Since the initial implementation of TypeGuard, there have been several fixes quickly applied to make mypy not crash on various TypeGuard things. This includes #10496, #10683 and #11015. We'll discuss how this PR relates to each of these three changes.

In particular, #10496 seems incorrect. As A5rocks discusses in #10899 , it introduces confusion between a type guarded variable and a TypeGuard[T]. This PR basically walks back that change entirely and renames TypeGuardType to TypeGuardedType to reduce that possible confusion.

Now, we still have the issue that TypeGuardedTypes are getting everywhere and causing unhappiness. I see two high level solutions to this:
a) Make TypeGuardedType a proper type, then delegate to the wrapped type in a bunch of type visitors and arbitrary amounts of other places where multiple types interact, and hope that we got all of them,
b) Make TypeGuardedType as an improper type (as it was in the original implementation)! Similar to TypeAliasType, it's just a wrapper for another type, so we unwrap it in get_proper_type. This is the approach this PR takes. This might feel controversial, but I think it could be the better option. It also means that if we type check we won't get type guard crashes.

#10683 is basically "remove call that leads to crash from the stacktrace". I think the join here (that ends up being with the wrapped type of the TypeGuardedType) is actually fine: if it's different, it tells us that the type changed, which is what we want to know. So seems fine to remove the special casing.

Finally, #11015. This is the other contentious part of this PR. I liked the idea of moving the core "type guard overrides narrowing" idea into meet.py, so I kept that. But my changes ended up regressing a reveal_type testTypeGuardNestedRestrictionAny test that was added. But it's not really clear to me how that worked or really, what it tested. I tried writing a simpler version of what I thought the test was meant to test (this is testTypeGuardMultipleCondition added in this PR), but that fails on master.

Anyway, this should at least fix the type guard crashes that have been coming up.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants