pyodbc icon indicating copy to clipboard operation
pyodbc copied to clipboard

Typing information installed to the wrong location

Open kadler opened this issue 4 years ago • 10 comments

Please first make sure you have looked at:

  • Documentation: https://github.com/mkleehammer/pyodbc/wiki
  • Other issues

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.

kadler avatar Jul 07 '21 18:07 kadler

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

keitherskine avatar Jul 08 '21 03:07 keitherskine

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.

kadler avatar Jul 08 '21 06:07 kadler

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?

keitherskine avatar Jul 10 '21 22:07 keitherskine

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.

kadler avatar Jul 12 '21 14:07 kadler

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.

keitherskine avatar Jul 14 '21 02:07 keitherskine

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

jpz avatar Oct 17 '21 09:10 jpz

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.

ajnelson-nist avatar Nov 15 '21 19:11 ajnelson-nist

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.

mdryden avatar May 30 '22 21:05 mdryden

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

JasonMendoza2008 avatar Aug 24 '22 21:08 JasonMendoza2008

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: https://github.com/pypa/setuptools/discussions/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.

kadler avatar Aug 25 '22 15:08 kadler

Fixed with #1146 .

keitherskine avatar Jan 08 '23 03:01 keitherskine