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 suite is not run with strict mode and many typing errors still remain #1377

Closed
marcinbarczynski opened this issue Dec 12, 2023 · 11 comments
Labels
bug Something isn't working pep 484 typing related issues

Comments

@marcinbarczynski
Copy link

marcinbarczynski commented Dec 12, 2023

Describe the bug

In a basic checking mode, pyright reports an error when importing a symbol from a module that is not re-exported using import ... as ... convention. In alembic.util package __init__.py imports symbols from other modules within the package.
I assume the intention was to re-export the imported symbols.

Expected behavior

No error reported by pyright.

To Reproduce

from alembic.util import coerce_resource_to_filename

def fun():
    coerce_resource_to_filename("A")

Error

$ pyright ../demo.py 
/path/to/demo.py
  /path/to/demo.py:1:26 - error: "coerce_resource_to_filename" is not exported from module "alembic.util"
    Import from "alembic.util.pyfiles" instead (reportPrivateImportUsage)
1 error, 0 warnings, 0 informations 

Versions.

  • OS: Linux
  • Python: 3.11.5
  • pyright: 1.1.339
  • Alembic: 1.13.0

Have a nice day!

@marcinbarczynski marcinbarczynski added the requires triage New issue that requires categorization label Dec 12, 2023
@zzzeek
Copy link
Member

zzzeek commented Dec 12, 2023

hi -

I'm not able to get pyright to report an error on the script above (nor mypy). I am trying to put pyright into fully strict mode as well:

pyrightconfig.json

{
    "typeCheckingMode": "strict"
}

no errors:

$ PYTHONPATH=~/dev/sqlalchemy/lib/ pyright  test3.py 
0 errors, 0 warnings, 0 informations 

I agree that type checkers often complain about imports not being exported but that is not happening here for either tool, and we most certainly do have a full strict mypy run for alembic in CI which passes.

@zzzeek zzzeek closed this as completed Dec 12, 2023
@zzzeek zzzeek added cant reproduce pep 484 typing related issues and removed requires triage New issue that requires categorization labels Dec 12, 2023
@zzzeek zzzeek reopened this Dec 12, 2023
@zzzeek
Copy link
Member

zzzeek commented Dec 12, 2023

wow did not intend to close this, sorry!!

@zzzeek
Copy link
Member

zzzeek commented Dec 12, 2023

cc @CaselIT , you might be more familiar with the rules for how exported symbols work w typing

@CaselIT
Copy link
Member

CaselIT commented Dec 12, 2023

Isn't it the fact that they insist on having stuff exported with from bar import foo as foo?

I'm not too sure if all things in alembic.util should be considered public though

@zzzeek
Copy link
Member

zzzeek commented Dec 12, 2023

sure, just want to understand what the rules are because we definitely had to add these exports in sqlalchemy

@marcinbarczynski
Copy link
Author

I'm not able to get pyright to report an error on the script above

I'm surprised you can't reproduce it.

These are the step to reproduce from scratch:

$ mkvirtualenv demo
(demo) $ pip install pyright
(demo) $ pip install alembic
(demo) $ cat > demo.py <<EOF
> from alembic.util import coerce_resource_to_filename

def fun():
    coerce_resource_to_filename("A")
> EOF
(demo) $ pyright demo.py
/tmp/demo.py
  /tmp/demo.py:1:26 - error: "coerce_resource_to_filename" is not exported from module "alembic.util"
    Import from "alembic.util.pyfiles" instead (reportPrivateImportUsage)
1 error, 0 warnings, 0 informations 

In strict mode:

(demo) $ cat >pyrightconfig.json <<EOF
> {
    "typeCheckingMode": "strict"
}
> EOF
(demo) $ pyright -p pyrightconfig.json demo.py
/tmp/demo.py
  /tmp/demo.py:1:26 - error: "coerce_resource_to_filename" is not exported from module "alembic.util"
    Import from "alembic.util.pyfiles" instead (reportPrivateImportUsage)
1 error, 0 warnings, 0 informations 

The same result on two separate machines.

@dybi
Copy link

dybi commented Dec 13, 2023

I concur - reproduced on
22.04.1-Ubuntu with Python 3.10.12
alembic 1.13.0
pyright 1.1.340

@zzzeek
Copy link
Member

zzzeek commented Dec 13, 2023

do the rules for typing change when I am running pyright from within the source tree of alembic itself? I've looked all around to see if I have any pyright configs (or mypy configs) that would cancel it out.

Here's a run from an editable install, I added newlines between my commands and you can see the output

[classic@framework tmp]$ git clone https://github.com/sqlalchemy/alembic
Cloning into 'alembic'...
remote: Enumerating objects: 14513, done.
remote: Counting objects: 100% (1714/1714), done.
remote: Compressing objects: 100% (154/154), done.
remote: Total 14513 (delta 1578), reused 1578 (delta 1552), pack-reused 12799
Receiving objects: 100% (14513/14513), 3.72 MiB | 31.19 MiB/s, done.
Resolving deltas: 100% (10451/10451), done.

[classic@framework tmp]$ cd alembic/

[classic@framework alembic:main]$ cat > demo.py <<EOF
> from alembic.util import coerce_resource_to_filename
> def fun():
    coerce_resource_to_filename("A")
> EOF

[classic@framework alembic:main]$ virtualenv .venv
created virtual environment CPython3.12.0.final.0-64 in 83ms
  creator CPython3Posix(dest=/home/classic/tmp/alembic/.venv, clear=False, no_vcs_ignore=False, global=False)
  seeder FromAppData(download=False, pip=bundle, via=copy, app_data_dir=/home/classic/.local/share/virtualenv)
    added seed packages: pip==23.3.1
  activators BashActivator,CShellActivator,FishActivator,NushellActivator,PowerShellActivator,PythonActivator

[classic@framework alembic:main]$ .venv/bin/pip install -e .
Obtaining file:///home/classic/tmp/alembic
  Installing build dependencies ... done
  Checking if build backend supports build_editable ... done
  Getting requirements to build editable ... done
  Installing backend dependencies ... done
  Preparing editable metadata (pyproject.toml) ... done
Collecting SQLAlchemy>=1.3.0 (from alembic==1.13.1.dev0)
  Using cached SQLAlchemy-2.0.23-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (9.6 kB)
Collecting Mako (from alembic==1.13.1.dev0)
  Using cached Mako-1.3.0-py3-none-any.whl.metadata (2.9 kB)
Collecting typing-extensions>=4 (from alembic==1.13.1.dev0)
  Using cached typing_extensions-4.9.0-py3-none-any.whl.metadata (3.0 kB)
Collecting greenlet!=0.4.17 (from SQLAlchemy>=1.3.0->alembic==1.13.1.dev0)
  Using cached greenlet-3.0.2-cp312-cp312-manylinux_2_24_x86_64.manylinux_2_28_x86_64.whl.metadata (3.7 kB)
Collecting MarkupSafe>=0.9.2 (from Mako->alembic==1.13.1.dev0)
  Using cached MarkupSafe-2.1.3-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl.metadata (2.9 kB)
Using cached SQLAlchemy-2.0.23-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (3.2 MB)
Using cached typing_extensions-4.9.0-py3-none-any.whl (32 kB)
Using cached Mako-1.3.0-py3-none-any.whl (78 kB)
Using cached greenlet-3.0.2-cp312-cp312-manylinux_2_24_x86_64.manylinux_2_28_x86_64.whl (621 kB)
Using cached MarkupSafe-2.1.3-cp312-cp312-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (28 kB)
Building wheels for collected packages: alembic
  Building editable for alembic (pyproject.toml) ... done
  Created wheel for alembic: filename=alembic-1.13.1.dev0-0.editable-py3-none-any.whl size=6643 sha256=1937db6bed089b82324d9cb693a384a9a1ac9401c363114fb2a3b9e3b56367bd
  Stored in directory: /tmp/pip-ephem-wheel-cache-pa8ba87q/wheels/06/ef/a0/2d7a25e036fc488876ab25c0fc0386326a019996c54cccdb2f
Successfully built alembic
Installing collected packages: typing-extensions, MarkupSafe, greenlet, SQLAlchemy, Mako, alembic
Successfully installed Mako-1.3.0 MarkupSafe-2.1.3 SQLAlchemy-2.0.23 alembic-1.13.1.dev0 greenlet-3.0.2 typing-extensions-4.9.0

[classic@framework alembic:main]$ .venv/bin/pip install pyright
Collecting pyright
  Downloading pyright-1.1.340-py3-none-any.whl.metadata (6.2 kB)
Collecting nodeenv>=1.6.0 (from pyright)
  Downloading nodeenv-1.8.0-py2.py3-none-any.whl.metadata (21 kB)
Collecting setuptools (from nodeenv>=1.6.0->pyright)
  Using cached setuptools-69.0.2-py3-none-any.whl.metadata (6.3 kB)
Downloading pyright-1.1.340-py3-none-any.whl (18 kB)
Downloading nodeenv-1.8.0-py2.py3-none-any.whl (22 kB)
Using cached setuptools-69.0.2-py3-none-any.whl (819 kB)
Installing collected packages: setuptools, nodeenv, pyright
Successfully installed nodeenv-1.8.0 pyright-1.1.340 setuptools-69.0.2

[classic@framework alembic:main]$ .venv/bin/pyright demo.py 

added 1 package, and audited 2 packages in 1s

found 0 vulnerabilities
0 errors, 0 warnings, 0 informations 
[classic@framework alembic:main]$ 


it does reproduce with a non-editable install and plain venv though

@zzzeek
Copy link
Member

zzzeek commented Dec 13, 2023

it looks like for mypy we have not yet turned on strict mode in our pyproject.toml, so that's really what has to be fixed here

@zzzeek zzzeek changed the title pyright complains about: "from alembic.util import coerce_resource_to_filename": reportPrivateImportUsage mypy suite is not run with strict mode and many typing errors still remain Dec 13, 2023
@zzzeek zzzeek added the bug Something isn't working label Dec 13, 2023
@zzzeek
Copy link
Member

zzzeek commented Dec 13, 2023

additionally there is redundant mypy config in setup.cfg vs pyproject.toml, remove the setup.cfg part

@sqla-tester
Copy link
Collaborator

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

finish strict typing for most modules https://gerrit.sqlalchemy.org/c/sqlalchemy/alembic/+/5016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pep 484 typing related issues
Projects
None yet
Development

No branches or pull requests

5 participants