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

Typing information installed to the wrong location #925

Closed
kadler opened this issue Jul 7, 2021 · 11 comments
Closed

Typing information installed to the wrong location #925

kadler opened this issue Jul 7, 2021 · 11 comments

Comments

@kadler
Copy link
Contributor

kadler commented Jul 7, 2021

Please first make sure you have looked at:

Environment

To diagnose, we usually need to know the following, including version numbers. On Windows, be
sure to specify 32-bit Python or 64-bit:

  • Python: 3.9
  • pyodbc: 4.0.31
  • OS: Linux
  • DB: N/A
  • driver: N/A

Issue

Python type stubs added in #864 are installed to the incorrect location:

    /home/kadler/venv-temp/lib/python3.9/site-packages/pyodbc-4.0.31.dist-info/*
    /home/kadler/venv-temp/lib/python3.9/site-packages/pyodbc.cpython-39-x86_64-linux-gnu.so
    /home/kadler/venv-temp/pyodbc.pyi

pyodbc.pyi should be installed along pyodbc.so in site-packages, but is installed in the base of the venv/prefix.

@keitherskine
Copy link
Collaborator

Rats, looks like data_files wasn't such a great idea. Thanks for pointing this out.

@kadler
Copy link
Contributor Author

kadler commented Jul 8, 2021

I did some investigation to figure out the right way to do it, but it seems that package_data wants .py files not C extensions. Outside my setuptools knowledge, unfortunately.

@keitherskine
Copy link
Collaborator

Actually, on further investigation, it looks like the data_files approach might be the best available, even though it's not ideal. Yes, it causes the pyodbc.pyi file to be placed at the top level of the virtual environment, but at least that's consistent whether it's Windows/Linux or source/wheel distribution (as far as I can tell). Besides, linters should still be able to find it. With Pylance on VS Code (on Windows), its search path for .pyi files includes the venv top-level directory by default.

I agree the comment in setup.py (line 94, near "data_files") is incorrect though, and should be updated at some point.

So, just to be clear, @kadler, are you having a specific issue with the location of pyodbc.pyi? If so, what would that be?

@kadler
Copy link
Contributor Author

kadler commented Jul 12, 2021

For a venv it is a bit odd, but it works I guess. The problem is more important if you're not installing in to a venv, though, since it gets installed in to the Python installation prefix (sys.prefix, eg. /usr). Having random files in this path is not ideal and I don't think PyODBC should be doing that. Especially problematic is if you have multiple versions of Python installed all using the same prefix, this file will be overwritten.

@keitherskine
Copy link
Collaborator

I agree the current approach is not ideal, but I'm struggling to come up with something better. Unfortunately, Python packaging is still not great, even on version 3.9. Including random files in non-package distributions just isn't well supported at the moment.

One potential approach might be to add specific package info to setup.py, something like this:

'packages': ['pyodbc'],
'package_dir': {'pyodbc': 'src'},
'package_data': {'pyodbc': ['src/pyodbc.pyi']},
'include_package_data': True,

This does appear to add the src directory files to both wheel and source distributions (including pyodbc.pyi). However, it seems quite a significant change to setup.py and I'm not sure what the downstream ramifications would be in all scenarios.

The fallback solution is not to package pyodbc.pyi in the distribution at all. Instead, add it to typeshed, but I was kinda hoping to keep it in-house for ease of maintenance.

@jpz
Copy link

jpz commented Oct 17, 2021

Just fyi, this is where pip has installed pyodbc.pyi in my docker container - in /usr/local/pyodbc.pyi. I was just reading this thread and trying to diagnose why VSCode/Pylance was not working for pyodbc.

Thanks for having worked on this @keitherskine - it's valuable work. I hope this data point is useful.

root@3179bde9bb8d:/usr# find . | grep pyodbc
./local/lib/python3.9/site-packages/pyodbc-4.0.32.dist-info
./local/lib/python3.9/site-packages/pyodbc-4.0.32.dist-info/REQUESTED
./local/lib/python3.9/site-packages/pyodbc-4.0.32.dist-info/LICENSE.txt
./local/lib/python3.9/site-packages/pyodbc-4.0.32.dist-info/WHEEL
./local/lib/python3.9/site-packages/pyodbc-4.0.32.dist-info/top_level.txt
./local/lib/python3.9/site-packages/pyodbc-4.0.32.dist-info/RECORD
./local/lib/python3.9/site-packages/pyodbc-4.0.32.dist-info/METADATA
./local/lib/python3.9/site-packages/pyodbc-4.0.32.dist-info/INSTALLER
./local/lib/python3.9/site-packages/pyodbc.cpython-39-x86_64-linux-gnu.so
./local/pyodbc.pyi

@ajnelson-nist
Copy link
Contributor

FWIW, with how the stubs file is currently installed directly into ${prefix}, mypy does not recognize that the stubs exist. They can still be used if an extra argument is provided to mypy, ${VIRTUAL_ENV}/pyodbic.pyi, but I suspect this is not the intended direction for users' type checking calls.

I found this usage issue along the way to filing PR 979.

@mdryden
Copy link

mdryden commented May 30, 2022

For anyone looking for a workaround here (sorry, I can't help the library authors), adding the install location as an extra python path to the VSCode settings resolves the problem for me.

For example, mine was installed in the root of my virtual environment (installed in ./.venv), and adding this to my settings.json file, then reloading the IDE resolves the import for me:

    "python.analysis.extraPaths": [
        ".venv"
    ]

If you installed without a virtual environment I'm pretty certain that using the absolute path to the installation would work as well. I used @jpz 's command from above to locate mine.

@JasonMendoza2008
Copy link

Not sure if it's linked, but here is a stackoverflow post that might be linked.

@kadler
Copy link
Contributor Author

kadler commented Aug 25, 2022

From what I remember, setuptools can do this correctly using package_data, but that wants a Python module/package, not a C extension.

I've started a discussion over in the setuptools group to see if they have any suggestions: pypa/setuptools#3563

One simple hack to fix this would be to create a Python shim for the C extension, eg.

src/pyodbc.py

from _pyodbc import *

Not sure how well that change would be received, however.

@keitherskine
Copy link
Collaborator

Fixed with #1146 .

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

No branches or pull requests

6 participants