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

python setup.py develop fails with version = attr: pkg.__version__ in setup.cfg #1724

Closed
kohr-h opened this issue Mar 19, 2019 · 16 comments
Closed

Comments

@kohr-h
Copy link

kohr-h commented Mar 19, 2019

See also pypa/pip#6350

When a package has version = attr: pkg.__version__ in its setup.cfg file, python setup.py develop can fail with a ModuleNotFoundError if not all runtime dependencies have been installed yet.

I've created a minimal repo to reproduce the error: https://github.com/kohr-h/minimal

Reproducing the error:

  • Create a fresh environment with python inside, but without numpy (our example dependency)
  • Run python setup.py develop in the repo root

Workaround:

  • Install numpy as well
  • Now python setup.py develop succeeds

Traceback

Traceback (most recent call last):
  File "setup.py", line 3, in <module>
    setup()
  File "/home/hkohr/miniconda/envs/tmp/lib/python3.7/site-packages/setuptools/__init__.py", line 145, in setup
    return distutils.core.setup(**attrs)
  File "/home/hkohr/miniconda/envs/tmp/lib/python3.7/distutils/core.py", line 121, in setup
    dist.parse_config_files()
  File "/home/hkohr/miniconda/envs/tmp/lib/python3.7/site-packages/setuptools/dist.py", line 705, in parse_config_files
    ignore_option_errors=ignore_option_errors)
  File "/home/hkohr/miniconda/envs/tmp/lib/python3.7/site-packages/setuptools/config.py", line 120, in parse_configuration
    meta.parse()
  File "/home/hkohr/miniconda/envs/tmp/lib/python3.7/site-packages/setuptools/config.py", line 425, in parse
    section_parser_method(section_options)
  File "/home/hkohr/miniconda/envs/tmp/lib/python3.7/site-packages/setuptools/config.py", line 398, in parse_section
    self[name] = value
  File "/home/hkohr/miniconda/envs/tmp/lib/python3.7/site-packages/setuptools/config.py", line 183, in __setitem__
    value = parser(value)
  File "/home/hkohr/miniconda/envs/tmp/lib/python3.7/site-packages/setuptools/config.py", line 513, in _parse_version
    version = self._parse_attr(value, self.package_dir)
  File "/home/hkohr/miniconda/envs/tmp/lib/python3.7/site-packages/setuptools/config.py", line 348, in _parse_attr
    module = import_module(module_name)
  File "/home/hkohr/miniconda/envs/tmp/lib/python3.7/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
  File "<frozen importlib._bootstrap>", line 983, in _find_and_load
  File "<frozen importlib._bootstrap>", line 967, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 677, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/hkohr/git/minimal/minimal/__init__.py", line 2, in <module>
    from . import mod
  File "/home/hkohr/git/minimal/minimal/mod.py", line 1, in <module>
    import numpy
ModuleNotFoundError: No module named 'numpy'

I wonder whether this can be fixed elegantly and does not sit too deep.

@pganssle
Copy link
Member

@kohr-h First of all, thank you for the thorough and easy-to-reproduce bug report, and sorry for the delay in responding.

Unfortunately, I do not think that it can be fixed at all, much less elegantly. By pulling the version attribute from the package, you need to be able to import the package itself as part of the build, which means you now have as a build-time dependency all the runtime dependencies that are imported when importing your package. I do not think there's anything we can do without actually changing import semantics, and it's not a workflow we're particularly keen to support anyway.

I have verified that this still breaks with pip install . and pip install -e ., which are the preferred ways to install / develop packages these days.

I think your best options are to either try a different method of single-sourcing your versions, there are several listed here (though I can't say I recommend many of them, the first one in particular sounds horrible). One common option is to store the version in source control (e.g. git tags, see setuptools_scm) and generate a _version.py file as part of the build. That's also very easy to recreate yourself by having some VERSION file in the repo and generating a corresponding _version.py at build time.

You could also add all your relevant runtime dependencies as build-time dependencies in your pyproject.toml file, though I would not recommend that, since it seems excessive to install all of those dependencies (not to mention maintain the list in two places) just to avoid duplicating your version number.

I hope that this has been helpful!

@kohr-h
Copy link
Author

kohr-h commented Apr 29, 2019

@pganssle Thanks a lot for your detailed reply. I understand that the issue sits too deep and would require too far-reaching changes to be resolved.
I would have loved to use setuptools_scm, but since our release branches are stale, that tool infers the wrong version. Instead, I'll probably go for a hand-written VERSION text file that is included as

version = file: VERSION

in setup.cfg (with setuptools >=39.2), and read as string in the package __init__.py.

I'll close the issue.

@greschd
Copy link

greschd commented May 12, 2020

I have accidentally stumbled upon a workaround for this issue:

  • Create a separate _version.py at the top-level of the module, containing (only) __version__ = 'x.y.z'
  • In the top-level __init__.py, import before any other dependencies from ._version import __version__
  • Use attr: mypkg._version.__version__ in setup.cfg.

The reason this works is because the install process parses setup.cfg twice, first for the setup requirements, and then again when running the setup. The first import of mypkg._version will fail (because importing mypkg raises), but leave behind mypkg._version in sys.modules. The second import then succeeds.

Given that the workaround relies on this specific implementation detail, I can't say I would necessarily recommend this approach.

@bsolomon1124
Copy link

Confused here, since setuptools docs read:

setuptools 46.4.0 added rudimentary AST analysis so that attr: can function without having to import any of the package’s dependencies.

Does that not apply to pip install [-e] .? Perhaps that should be a disclaimer, since @greschd 's band-aid works; if you have:

pkg/
-- __init__.py
-- _version.py

Where _version.py defines __version__ == '1.0.0' and that is imported into __init.py__ after any third-party imports, then setup will fail, but placing the import before any third-party deps makes it work. Isn't the point of the AST approach to avoid things like this?

@greschd
Copy link

greschd commented Sep 29, 2020

Yeah, I'm confused by that also. As far as I can tell, it's not limited to editable installs: pip install . also fails, as well as installing from a sdist. It seems only wheels work in this scenario (which makes sense, they have all the metadata already "baked in").

I think this is the relevant PR: #1753

@greschd
Copy link

greschd commented Sep 29, 2020

After looking into that code a little bit, I think what does work is just assigning __version__ = "x.y.z" directly in the __init__.py, with attr: module_name.__version__.

The AST parsing only looks for assignments at the module top-level, and falls back on loading the module.

@bsolomon1124
Copy link

bsolomon1124 commented Sep 29, 2020

After looking into that code a little bit, I think what does work is just assigning __version__ = "x.y.z" directly in the __init__.py, with attr: module_name.__version__.

The AST parsing only looks for assignments at the module top-level, and falls back on loading the module.

As far as I know, the attr also allows specification of a specific module rather than using the package name (implicitly __init__.py)

Working example from @pganssle 's own:

https://github.com/pganssle/zoneinfo/blob/ceea631bea8d68662c6661f9027b98eede209790/setup.cfg#L3

But perhaps that's because zoneinfo has no third-party deps.

@bsolomon1124
Copy link

#1753 looks to have been incorporated in release 46.4.0 https://setuptools.readthedocs.io/en/latest/history.html#v46-4-0, which specifically mentions

thereby supporting modules with third-party imports

So, yeah, confused all around 😦

@pganssle
Copy link
Member

pganssle commented Sep 29, 2020

Working example from @pganssle 's own:

https://github.com/pganssle/zoneinfo/blob/ceea631bea8d68662c6661f9027b98eede209790/setup.cfg#L3

But perhaps that's because zoneinfo has no third-party deps.

Note that in zoneinfo, the attr: directive points to backports.zoneinfo._version.__version__, and not to backports.zoneinfo.__version__. backports.zoneinfo._version contains a single literal assignment and doesn't require resolving any imports, which is likely why the static AST parsing logic works (though in that case I think it's likely that an "exec the file" approach would also work, because the file doesn't have any third-party dependencies and isn't broken).

It seems that as long as you either 1. have no third party imports or 2. point the attr: directive at a literal __version__ = ... assignment rather than a derived quantity or something imported, this should just work.

@bsolomon1124
Copy link

bsolomon1124 commented Sep 29, 2020

It seems that as long as you either 1. have no third party imports or 2. point the attr: directive at a literal version = ... assignment rather than a derived quantity or something imported, this should just work.

Yes, though one small correction - those two conditions need a boolean-AND, not or :). I'm going to venture that 90% of packages that declare a simple one-liner in a module outside of __init__.py turn around and try to import that into __init__.py so that they can bring it directly under the package namespace.

Example:

mkdir pypa-1724
cd pypa-1724
mkdir -p src/package_a
touch README.md
echo '__version__ = "1.0.0"' > src/package_a/_version.py
echo 'import setuptools; setuptools.setup()' > setup.py

cat << EOF > src/package_a/__init__.py
from flask import Flask
from ._version import __version__
EOF

cat << EOF > setup.cfg
[metadata]
name = package-a
version = attr:package_a._version.__version__
description = foo
long_description = file: README.md
long_description_content_type = text/markdown
license = Apache-2.0

[options]
packages = find:
install_requires =
    flask
    importlib_resources;python_version<"3.7"
include_package_data = True
package_dir =
    =src

[options.packages.find]
where = src
EOF

Then:

python3 -m venv venv
. ./venv/bin/activate
python3 -m pip install -e .

Breaks with:

ModuleNotFoundError: No module named 'flask'

whereas removing the flask import in __init__.py (and optionally dropping from install_requires) succeeds, verifiable with python3 -c 'import package_a; print(package_a.__version__)'.

@greschd
Copy link

greschd commented Sep 29, 2020

The problem here appears to be that setuptools tries to load the "parent module" before AST - analyzing _version.py. Not sure why it needs to do that.

Moving it to __init__.py seems to work, however:

mkdir pypa-1724
cd pypa-1724
mkdir -p src/package_a
touch README.md
echo 'import setuptools; setuptools.setup()' > setup.py

cat << EOF > src/package_a/__init__.py
from flask import Flask
__version__ = "1.0.0"
EOF

cat << EOF > setup.cfg
[metadata]
name = package-a
version = attr:package_a.__version__
description = foo
long_description = file: README.md
long_description_content_type = text/markdown
license = Apache-2.0

[options]
packages = find:
install_requires =
    flask
    importlib_resources;python_version<"3.7"
include_package_data = True
package_dir =
    =src

[options.packages.find]
where = src
EOF

@bsolomon1124
Copy link

The problem here appears to be that setuptools tries to load the "parent module" before AST - analyzing _version.py. Not sure why it needs to do that.

Right - I am probably glossing over some details from #1753, but that changelog entries reads as if this was the exact situation that is avoided by the ast-based finder.

drewejohnson pushed a commit to CORE-GATECH-GROUP/hydep that referenced this issue Nov 12, 2020
Very nice feature, but causes problems with fresh installs.
Essentially causes the package to be imported, which trickles
down to a numpy and/or scipy import. These are listed in the
install requires section, but the version search is done first.
Placing the explicit version in the setup.cfg means this has
to be updated at releases, but makes a completely fresh install
possible.

Related GH issues / workarounds:
pypa/setuptools#1724
pypa/pip#6350
voanhduy1512 pushed a commit to voanhduy1512/eks-rolling-update that referenced this issue Jan 24, 2021
felker added a commit to felker/balsam that referenced this issue Feb 22, 2021
setuptools >= 46.4.0 has rudimentary AST analysis for
attr: balsam.__version
to work without importing balsam (and hence possibly failing due to
missing external dependencies imported in __init__.py)

But __version__ must be in __init__.py and must be a hardcoded string
pypa/setuptools#1724
jaemolihm added a commit to jaemolihm/wannier-berri that referenced this issue Mar 22, 2021
jaemolihm added a commit to jaemolihm/wannier-berri that referenced this issue Mar 22, 2021
GardenTools added a commit to GardenTools/CrcEngine that referenced this issue Apr 9, 2021
Addressing problem caused by pypa/setuptools#1724
Accessing the version attribute could cause odd dependencies on import
statement ordering as to whether importing succeeded or failed
@GDYendell
Copy link

This works if I edit @kohr-h's minimal example to add the full path to __version__:

version = attr: minimal._version.__version__
(venv) /tmp/minimal ▶ pip list
Package    Version
---------- -------
pip        18.1
setuptools 40.6.2
(venv) /tmp/minimal ▶ python setup.py --version
0.1.0dev0

I guess AST requires this full path to import __version__ without relying on __init__.py, which will import numpy.

It doesn't, however, work for importing classes, such as for cmd_class.

@ThomasTNO
Copy link

For future reference, I managed to work around this error by using attr: pkg.__init__.__version__ instead of attr: pkg.__version__.

@mivade
Copy link

mivade commented Nov 30, 2021

This doesn't appear to work with namespace packages. I tried @ThomasTNO's workaround but no luck.

ianhelle added a commit to microsoft/msticpy that referenced this issue Jan 18, 2022
…all -e .

This is because setuptools tries to import the msticpy package before dependencies are fully installed. See pypa/setuptools#1724
ianhelle added a commit to microsoft/msticpy that referenced this issue Jan 26, 2022
* Moving most setup config to setup.cfg

Extras couldn't be moved there since we have multiple overlapping extra groups.
Updated import_analyzer.py to avoid creating copy of setup.py

* Add Pipfile creation to create_reqs_all.py

* Trying to get the version in setup.cfg causes a failure with pip install -e .

This is because setuptools tries to import the msticpy package before dependencies are fully installed. See pypa/setuptools#1724

Co-authored-by: Pete Bryan <[email protected]>
@glroman
Copy link

glroman commented Mar 23, 2022

With setuptools==46.1.3, I got this to work. In setup.cfg:

[metadata]
    name             = my_mod
    version          = attr: my_mod.__version__

In my init.py:

__version__ = "1.2.3"

# Protect version extraction by setuptools (attr import)
_ok = True
try:
    import requests
except:
    print("Please install requests (if you haven't already)")
    _ok = False

if _ok:
    from .my_mod import Whatever

juju4 pushed a commit to juju4/msticpy that referenced this issue Jun 5, 2022
* Moving most setup config to setup.cfg

Extras couldn't be moved there since we have multiple overlapping extra groups.
Updated import_analyzer.py to avoid creating copy of setup.py

* Add Pipfile creation to create_reqs_all.py

* Trying to get the version in setup.cfg causes a failure with pip install -e .

This is because setuptools tries to import the msticpy package before dependencies are fully installed. See pypa/setuptools#1724

Co-authored-by: Pete Bryan <[email protected]>
m-julian added a commit to popelier-group/ichor that referenced this issue Jul 5, 2022
* using attr: can cause issues when other dependencies are not installed in the environment. If all of the dependencies are installed, then installation is successful (but that requires manual installation of all of the dependencies prior to installing ichor). Thus, for now put in the version manually instead of importing from __init__ file. See pypa/setuptools#1724
nvictus added a commit to open2c/cooler that referenced this issue Jan 16, 2023
nvictus added a commit to open2c/cooler that referenced this issue Jan 16, 2023
* Migrate setup.py completely to pyproject.toml

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Remove zipsafe from pyproject.toml

* Update pyproject.toml

* Try changing attr path
pypa/setuptools#1724

* Pin setuptools for PEP660 editable installs

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
stepan-tsirkin pushed a commit to wannier-berri/wannier-berri that referenced this issue Feb 20, 2024
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

8 participants