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

mypy 0.930: declarative_base() raises AssertionError #7496

Closed
uSpike opened this issue Dec 22, 2021 · 11 comments
Closed

mypy 0.930: declarative_base() raises AssertionError #7496

uSpike opened this issue Dec 22, 2021 · 11 comments
Labels
bug Something isn't working SQLA mypy plugin mypy plugin issues only. general pep-484 issues should be "typing"
Milestone

Comments

@uSpike
Copy link

uSpike commented Dec 22, 2021

Describe the bug

Running mypy version 0.930 on a python file with base = declarative_base() causes an internal error with mypy using the sqlalchemy.ext.mypy.plugin plugin.

from sqlalchemy.orm import declarative_base
base = declarative_base()

To Reproduce

$ python --version
Python 3.10.0
$ mypy --version
mypy 0.930
cat mypy.ini 
[mypy]
python_version=3.10
platform=linux
plugins=sqlalchemy.ext.mypy.plugin

Error

mypy mypy_test.py --show-traceback
mypy_test.py:3: 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.930
Traceback (most recent call last):
  File "mypy/semanal.py", line 5132, in accept
  File "mypy/nodes.py", line 1140, in accept
  File "mypy/semanal.py", line 2110, in visit_assignment_stmt
  File "mypy/semanal.py", line 2392, in apply_dynamic_class_hook
  File "/home/.../lib/python3.10/site-packages/sqlalchemy/ext/mypy/plugin.py", line 145, in _dynamic_class_hook
    obj = ctx.api.named_type("__builtins__.object")
  File "mypy/semanal.py", line 4538, in named_type
  File "mypy/semanal.py", line 4505, in lookup_fully_qualified
AssertionError: 
mypy_test.py:3: : note: use --pdb to drop into pdb

Versions

  • OS: Ubuntu 20.04
  • Python: 3.10.0
  • SQLAlchemy: 1.4.28
  • Database: N/A
  • DBAPI (eg: psycopg, cx_oracle, mysqlclient): N/A

Additional context

No response

@uSpike uSpike added the requires triage New issue that requires categorization label Dec 22, 2021
@zzzeek zzzeek added bug Something isn't working SQLA mypy plugin mypy plugin issues only. general pep-484 issues should be "typing" and removed requires triage New issue that requires categorization labels Dec 22, 2021
@zzzeek zzzeek added this to the 1.4.x milestone Dec 22, 2021
@zzzeek
Copy link
Member

zzzeek commented Dec 22, 2021

yes it looks like mypy 9.3.0 just came out 3 hours ago and we are hitting it also.

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the main branch:

use fully qualified, locatable names for all use of api.named_type() https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/3425

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the rel_1_4 branch:

use fully qualified, locatable names for all use of api.named_type() https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/3426

sqlalchemy-bot pushed a commit that referenced this issue Dec 22, 2021
Fixed mypy regression where the release of mypy 0.930 added additional
internal checks to the format of "named types", requiring that they be
fully qualified and locatable. This broke the mypy plugin for SQLAlchemy,
raising an assertion error, as there was use of symbols such as
``__builtins__`` and other un-locatable or unqualified names that
previously had not raised any assertions.

Fixes: #7496
Change-Id: I037680606a1d51158ef6503508ec76c5d5adc946
(cherry picked from commit aded8b1)
@vfazio
Copy link

vfazio commented Dec 27, 2021

Note that the change is not backwards compatible, so you cannot use the plugin from 1.4.29 with mypy .910

@zzzeek
Copy link
Member

zzzeek commented Dec 27, 2021

@vfazio do you have a source for that? if it just "breaks" in .910 we may have just done the change incorrectly. if OTOH mypy has published that the calling API of api.named_type() is now backwards incompatible from .910 to .930 such that you can't write a single call to it which works in both versions, that seems like it should be noted on their side somewhere and would be a little bit surprising.

@vfazio
Copy link

vfazio commented Dec 27, 2021

@vfazio do you have a source for that? if it just "breaks" in .910 we may have just done the change incorrectly. if OTOH mypy has published that the calling API of api.named_type() is now backwards incompatible from .910 to .930 such that you can't write a single call to it which works in both versions, that seems like it should be noted on their side somewhere and would be a little bit surprising.

Sure, I can demonstrate:

(xtf-310) vfazio@vfazio2 /mnt/development/xtf :( $ mypy --version 
mypy 0.910
(xtf-310) vfazio@vfazio2 /mnt/development/xtf $ python3 -c "import sqlalchemy; print(sqlalchemy.__version__)"
1.4.29
(xtf-310) vfazio@vfazio2 /mnt/development/xtf $ cat simple.py
from sqlalchemy import Column, DateTime, String
from sqlalchemy.orm import declarative_base

Base = declarative_base()


class Session(Base):
    __tablename__ = "session"

    id = Column(String, primary_key=True)
    
(xtf-310) vfazio@vfazio2 /mnt/development/xtf $ mypy --show-traceback simple.py 
simple.py:7: 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.910
Traceback (most recent call last):
  File "/home/vfazio/.pyenv/versions/xtf-310/bin/mypy", line 8, in <module>
    sys.exit(console_entry())
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/__main__.py", line 11, in console_entry
    main(None, sys.stdout, sys.stderr)
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/main.py", line 87, in main
    res, messages, blockers = run_build(sources, options, fscache, t0, stdout, stderr)
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/main.py", line 165, in run_build
    res = build.build(sources, options, None, flush_errors, fscache, stdout, stderr)
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/build.py", line 179, in build
    result = _build(
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/build.py", line 254, in _build
    graph = dispatch(sources, manager, stdout)
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/build.py", line 2697, in dispatch
    process_graph(graph, manager)
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/build.py", line 3021, in process_graph
    process_stale_scc(graph, scc, manager)
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/build.py", line 3113, in process_stale_scc
    mypy.semanal_main.semantic_analysis_for_scc(graph, scc, manager.errors)
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/semanal_main.py", line 78, in semantic_analysis_for_scc
    process_top_levels(graph, scc, patches)
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/semanal_main.py", line 199, in process_top_levels
    deferred, incomplete, progress = semantic_analyze_target(next_id, state,
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/semanal_main.py", line 326, in semantic_analyze_target
    analyzer.refresh_partial(refresh_node,
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/semanal.py", line 396, in refresh_partial
    self.refresh_top_level(node)
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/semanal.py", line 407, in refresh_top_level
    self.accept(d)
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/semanal.py", line 4872, in accept
    node.accept(self)
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/nodes.py", line 950, in accept
    return visitor.visit_class_def(self)
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/semanal.py", line 1056, in visit_class_def
    self.analyze_class(defn)
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/semanal.py", line 1134, in analyze_class
    self.analyze_class_body_common(defn)
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/semanal.py", line 1143, in analyze_class_body_common
    self.apply_class_plugin_hooks(defn)
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/semanal.py", line 1203, in apply_class_plugin_hooks
    hook(ClassDefContext(defn, base_expr, self))
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/sqlalchemy/ext/mypy/plugin.py", line 260, in _base_cls_hook
    decl_class.scan_declarative_assignments_and_apply_types(ctx.cls, ctx.api)
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/sqlalchemy/ext/mypy/decl_class.py", line 96, in scan_declarative_assignments_and_apply_types
    _scan_declarative_assignment_stmt(
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/sqlalchemy/ext/mypy/decl_class.py", line 481, in _scan_declarative_assignment_stmt
    apply.apply_type_to_mapped_statement(
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/sqlalchemy/ext/mypy/apply.py", line 210, in apply_type_to_mapped_statement
    left_node.type = api.named_type(
  File "/home/vfazio/.pyenv/versions/3.10.0/envs/xtf-310/lib/python3.10/site-packages/mypy/semanal.py", line 4310, in named_type
    assert sym, "Internal error: attempted to construct unknown type"
AssertionError: Internal error: attempted to construct unknown type
simple.py:7: : note: use --pdb to drop into pdb

//rerun with --pdb

(Pdb) qualified_name
'sqlalchemy.orm.attributes.Mapped'

(xtf-310) vfazio@vfazio2 /mnt/development/xtf $ python3 -c "import sqlalchemy; print(sqlalchemy.__version__)"
1.4.28
(xtf-310) vfazio@vfazio2 /mnt/development/xtf $ mypy --version
mypy 0.910
(xtf-310) vfazio@vfazio2 /mnt/development/xtf $ mypy --show-traceback simple.py 
Success: no issues found in 1 source file

I haven't gotten much further in debugging

@zzzeek
Copy link
Member

zzzeek commented Dec 27, 2021

Yeah I've no doubt you're getting a traceback like that. What we need is mypy devs to tell us how to use this method correctly. it shouldn't work by accident in one version one way, then another version in another way. getting mypy devs to clarify here is what we need.

@vfazio
Copy link

vfazio commented Dec 27, 2021

Apparently thats even with a hack i had in. I had changed the

obj = ctx.api.named_type(names.NAMED_TYPE_BUILTINS_OBJECT)

references to

obj = ctx.api.object_type()

to get those to shut up in .910 because of the change from __builtins__ to builtins, but I don't know if those are "legal" methods to use

python/mypy#6617 says "no guarantees about backwards compatibility"

@97littleleaf11
Copy link

97littleleaf11 commented Feb 20, 2022

I apologize for the regression caused by the change of related APIs. SemanticAnalyzer.named_type was inconsistent with the implementation of Checker.named_type thus I made a fix. I should have mentioned that in the API announcement issue. Sorry for all the trouble.

@zzzeek
Copy link
Member

zzzeek commented Feb 21, 2022

I apologize for the regression caused by the change of related APIs. SemanticAnalyzer.named_type was inconsistent with the implementation of Checker.named_type thus I made a fix. I should have mentioned that in the API announcement issue. Sorry for all the trouble.

OK is the API of this method now stable or do we have to fix it again?

@97littleleaf11
Copy link

The current SemanticAnalyzer.named_type API works the same as TypeChecker.named_type. We would refactor the internal implementation of lookup functions in the future, but i think it wouldn't cause a regression change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working SQLA mypy plugin mypy plugin issues only. general pep-484 issues should be "typing"
Projects
None yet
Development

No branches or pull requests

5 participants