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

Bad error message: mypy reports 'Name is not defined' when using an object member as super class of another class #8603

Open
sandrinr opened this issue Mar 30, 2020 · 24 comments

Comments

@sandrinr
Copy link

Consider the following valid Python 3 code:

class A():
    pass

class B():
    def __init__(self):
        self.a = A

b = B()

class C(b.a):
    pass

Actual behavior:
Mypy will report Name 'b.a' is not defined on the line class C(b.a):.

Expected behavior:
No error reported by Mypy.

Python version: 3.8.2
Mypy version: 0.770 (master not checked)
Flags: no flags

@msullivan
Copy link
Collaborator

This is an incorrect error message, certainly. We're not going to support this code, though.

@msullivan msullivan changed the title Mypy reports 'Name is not defined' when using an object member as super class of another class Bad error message: mypy reports 'Name is not defined' when using an object member as super class of another class Apr 3, 2020
@vhawk19
Copy link

vhawk19 commented Apr 19, 2020

@msullivan Should the error message be changed?

@JelleZijlstra
Copy link
Member

We'd be likely to accept a PR that improves the error message (something like "object member cannot be used as base class", I suppose).

@akash-suresh
Copy link

@JelleZijlstra This is my first time contributing. Can I pick this up?

@harshkumar02
Copy link

``I would like to contribute to this issue. Can I pull this up?

@PrasanthChettri
Copy link
Contributor

Can I look into this, seems like I can handle this one

@PrasanthChettri
Copy link
Contributor

PrasanthChettri commented Feb 14, 2021

@msullivan @JelleZijlstra I looked through the codebase where I can set up a check for this, I think I have to implement something around semanal/analyze base class or semanal/expr to analyzed type and have to check if the base class is a object member, I tried everything but I don't seem to know how to do this, could anybody guide me through this, I'm kind of a newbie.

@williamjmorenor
Copy link

Using per line comment as # type: ignore[name-defined] at less silence this error message

@SwagatSBhuyan
Copy link

If this issue is not solved, I would like to be assigned this @msullivan @JelleZijlstra

@kai3341
Copy link

kai3341 commented May 5, 2022

Have similar issue:

from typing import Type

class A: ...
class SomeMixin: ...
class some_framework_app: ...

def subclass_a(something):
    class A1(A):
        attr = something

    return A1


class B(some_framework_app):
    C: Type[A]
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.C = subclass_a(self)

app = B()

class D(app.C, SomeMixin): ...  # <=== Name "app.C " is not defined

@davidism
Copy link

davidism commented Sep 13, 2022

I'm currently adding annotations to Flask-SQLAlchemy. If this is not supported, I will never be able to enable type checking, as it breaks one of the major features of Flask-SQLAlchemy. If I do enable it, every project would have to add an ignore rule for name-defined to every model, or to their config, which is not ideal.

The extension creates a self.Model class that is used to register SQLAlchemy models with the extension instance. However, due to this issue, mypy shows an error for class User(db.Model).

Here's a minimal example without any SQLAlchemy-specific code:

import typing as t

class Example:
    Model: t.Type[object]

    def __init__(self):
        self.Model = object

db = Example()
reveal_type(db.Model)

class User(db.Model):
    pass
$ mypy --show-error-codes example.py
example.py:12: note: Revealed type is "Type[builtins.object]"
example.py:14: error: Name "db.Model" is not defined  [name-defined]

I don't understand why this won't be supported. Mypy already understands that the name is a Type, it should be possible to inherit from it.

@jace
Copy link

jace commented Nov 4, 2022

Flask-SQLAlchemy has blown up for me after the 3.0 release. As I can see from pallets-eco/flask-sqlalchemy#1118, the fix is to litter my code with # type: ignore comments. This is an unhappy state.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Nov 4, 2022

You could also

[mypy-flask_sqlalchemy.*]
follow_imports=skip

which would make it as if flask_sqlalchemy didn't have a py.typed, just like before v3

@davidism
Copy link

davidism commented Nov 4, 2022

But why would you want to do that? Most of the types are fine and helpful. It's just the fact that MyPy doesn't accept class Thing(db.Model) that's a problem. At least advise them to only ignore the name-defined error message, either from Flask-SQLAlchemy or within their model modules only.

@jace
Copy link

jace commented Nov 5, 2022

I'm removing Flask-SQLAlchemy from my code entirely. I've moved most references of db.* to sa.* and sa.orm.* so I could get type checking. Next step: replacing db.Model with a custom base model, then db.session.

This has been a nightmare year with perfectly functional code falling apart every few months as various framework libraries refactor themselves to fit into the constraints of static typing.

Static type checking has been an excellent development for Python – it's significantly reduced the need for test coverage of data types, or defensive code to evaluate function parameters – but it's also shrunk the dynamic nature of Python to the subset recognised by type checkers. My project now has an average of five # type: ignore lines per Python file, and I've spent way too much time this year on (a) researching ways to make an idiom type-compatible, and (b) giving up and adding an ignore.

An idiom as simple and helpful as this no longer works because None is not defined in the overloads for sa.Column:

class MyModel(ModelBase):
    fkey_id = sa.Column(None, sa.ForeignKey('other_table.id'))

Instead I have to lookup the other table, find the type of its primary key, and use:

class MyModel(ModelBase):
    fkey_id = sa.Column(sa.Integer, sa.ForeignKey('other_table.id'))

…or I throw in a # type: ignore and live with no static type checking for fkey_id wherever it's used downstream, which generally means that I have to read code that comes in for review very carefully.

Sorry for the rant. This period of transition is painful, but I'm hopeful for how this improves speed of development (at least post-transition) and code review.

taylormadore added a commit to containerbuildsystem/cachito that referenced this issue Feb 6, 2023
This is a workaround for how mypy reports a "name-defined"
error for all uses of db.Model.

See these for more information:
pallets-eco/flask-sqlalchemy#1118
python/mypy#8603

Signed-off-by: Taylor Madore <tmadore@redhat.com>
taylormadore added a commit to containerbuildsystem/cachito that referenced this issue Feb 8, 2023
This is a workaround for how mypy reports a "name-defined"
error for all uses of db.Model.

See these for more information:
pallets-eco/flask-sqlalchemy#1118
python/mypy#8603

Signed-off-by: Taylor Madore <tmadore@redhat.com>
@marcelm
Copy link

marcelm commented Mar 9, 2023

(via python/typeshed#9860)

Here’s an example of a seemingly innocuous usage of the multiprocessing module that triggers this error:

import multiprocessing

context = multiprocessing.get_context()

class MyProcess(context.Process):  # error: Name "context.Process" is not defined  [name-defined].
    pass

It would be great to hear why the restriction is in place. Is this just hard to solve technically or would it cause other issues were it allowed?

stsnel added a commit to UtrechtUniversity/yoda-external-user-service that referenced this issue Mar 17, 2023
Need to add ignores for the Flask-SQLAlchemy model declarations
as a workaround for limitations regarding inheritance from objects, see
python/mypy#8603
stsnel added a commit to UtrechtUniversity/yoda-external-user-service that referenced this issue Mar 17, 2023
Need to add ignores for the Flask-SQLAlchemy model declarations
as a workaround for limitations regarding inheritance from objects, see
python/mypy#8603
@BackSeat
Copy link

BackSeat commented Aug 5, 2023

@jace above expresses eloquently the frustration a lot of us share.

Mypy and Flask-SQLAlchemy are both popular packages. Would it be possible for the teams from each to talk to each other and find a better way forwards than either dropping type ignore everywhere or re-implementing the functionality offered by Flask-SQLAlchemy?

@davidism
Copy link

davidism commented Aug 5, 2023

Happy to review a PR to Flask-SQLAlchemy that fixes the issue while retaining compatibility with its existing interface. But there's nothing more to do on my end than what I've already commented here. Note that even if MyPy fixes it, you have to separately convince pyright (vscode) to fix it, and they've already declined.

@jace
Copy link

jace commented Aug 5, 2023

I've replaced db.Model with a custom base class and have full typing support now. It supports __bind_key__ too, but I chose to not implement the automatic __tablename__ construction as it felt a bit magical.

@BackSeat
Copy link

BackSeat commented Aug 5, 2023

That looks very interesting @jace, thank you. I will look into migrating to your solution.

@jace
Copy link

jace commented Aug 8, 2023

@davidism I'm sorry this is not drop-in compatible with Flask-SQLAlchemy or I'd have made a PR. I had to move all namespacing and base class config out of the SQLAlchemy instance to get typing to work, and there's a somewhat ugly two-line init to get Flask-SQLAlchemy's session management. My typing-fu only goes so far.

At best, I can think of a cleaner but backward-incompatible integration.

@BackSeat
Copy link

BackSeat commented Aug 8, 2023

This has been helpful discussion - thanks in particular to @davidism and @jace.

I think Flask-SQLAlchemy is a great extension for proof of concept code and for those less experienced with Flask and/or SQLAlchemy. As a project grows and the team become more experienced, tools such as mypy become more important and valuable. At some point the value of mypy takes precedent over the convenience of wrappers such as Flask-SQLAlchemy, particularly such wrappers don't sit well with mypy.

For now, we've adapted an approach outlined in this post, seemingly (and worryingly) only available on Wayback Machine. The approach is to define a declarative base outside of Flask-SQLAlchemy and then pass that to the extension, which not only avoids the mypy problems but also makes the declarative base available independently of Flask which, as it happens, is also helpful for us.

Some code changes were needed from those given in the post referenced above, and I'm happy to make them available on request.

For future projects, I suspect we'll choose not to use Flask-SQLAlchemy and just code SQLAlchemy access natively as documented on the Flask pages.

If Flask-SQLAlchemy were to take the approach of using a predefined declarative base then I suspect that would increase the utility of that extension. However, that would be backward incompatible and may not align with the future goals of Flask-SQLAlchemy.

@davidism
Copy link

davidism commented Aug 8, 2023

The reason I keep voting these comments down is because they're orthogonal to this issue and don't help fix the problem in MyPy or in Flask-SQLAlchemy. And it really, really sucks to be a maintainer that has poured countless hours into it to then be told "Flask-SQLAlchemy isn't good because it doesn't work with typing in this one case", or "Just rip out Flask-SQLAlchemy and replace it", or "I'm going to tell everyone not to use Flask-SQLAlchemy", or "Flask-SQLalchemy is only good for proofs of concept" etc. It really, really sucks. I am not happy with the fact that I keep hearing these things just because I need to watch this issue for actual resolutions to the issue.

I'd suggest not replying along these lines further, it's off topic here, sorry MyPy maintainers. But I couldn't remain silent in the face of being told my work isn't good over and over again, especially after being completely open to fixing the issue. And if you're response is "that wasn't my intention" it doesn't matter, that's how it came off anyway.

@FossenWang
Copy link

The db.Model is created dynamically for some reason, but Mypy need static type. I don't think it's either side's fault.
I like Flask-SQLAlchemy. The dynamic feature of python is definitely one of features that I much like. I don't want to sacrifice dynamic capability for static type.
I don't expect Mypy to support all popular dynamic patterns, but can we provide more actual use cases for those dynamic patterns?

Here is an example for Flask-SQLAlchemy, which works for me, but I'm just not sure if it's good practice:

from typing import TYPE_CHECKING
from flask_sqlalchemy import SQLAlchemy

db = SQLAlchemy()

if TYPE_CHECKING:
    from flask_sqlalchemy.model import Model
else:
    Model = db.Model


class ExampleModel(Model):
    pass


reveal_type(ExampleModel)
reveal_type(ExampleModel.query)

# note: Revealed type is "def () -> app.graphql.base.ExampleModel"
# note: Revealed type is "flask_sqlalchemy.query.Query"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests