Cannot build with `poetry-build` and habitat-sim as dependency. AssertionError on python version during setup.py execution.

bergercookie commented 2 years ago

Habitat-Sim version

Main branch.

🐛 Bug

On Ubuntu 18.04, with python3.6.9 as the system python version.
I'm using python-poetry to manage my python project, and habitat-sim is one of my project's dependencies.

I'm specifying this dependency with one of the following syntaxes in pyproject.toml:

python = "^3.7"
...
habitat-sim = { git = "https://github.com/facebookresearch/habitat-sim", branch = "v0.2.1" }

or having git cloned habitat-sim,

python = "^3.7"
...
habitat-sim = { path = "/path/to/habitat-sim/local/repo" }

When I have habitat-sim as a dependency, and because it's not published on PyPI so that its dependencies are known, I believe poetry has to download the package, create a temporary virtualenvironment, and do a local build, so that it can determine habitat-sim's dependencies, in order to then also fetch them to build my parent project.

However, when I'm running poetry install to do all the above, poetry fails with the message:

  PackageInfoError

  Unable to determine package info for path: /home/berger/slamcore/habitat-sim

  Fallback egg_info generation failed.

  Command ['/tmp/tmpltm7o1qr/.venv/bin/python', 'setup.py', 'egg_info'] errored with the following return code 1, and output:
  Traceback (most recent call last):
    File "setup.py", line 449, in <module>
      ) >= StrictVersion("3.7"), "Must use python3.7 or newer"
  AssertionError: Must use python3.7 or newer

This is because, in your setup.py, you specify:

if __name__ == "__main__":
    assert StrictVersion(
    ¦   "{}.{}".format(sys.version_info[0], sys.version_info[1])
    ) >= StrictVersion("3.7"), "Must use python3.7 or newer"
    with open("./requirements.txt", "r") as f:
    ¦   requirements = [l.strip() for l in f.readlines() if len(l.strip()) > 0]

Why is this check in place? The canonical way of mandating a python version for a package, is, and you already do this later in setup.py:

setup(
    name="habitat_sim",
    version=habitat_sim.__version__,
    ...
    python_requires=">=3.7",
    ....
)

Raising an AssertionError when the package is executed by an incompatible python version only disallows the introspection of the package dependencies.

Steps to Reproduce

Steps to reproduce the behavior:

Install poetry from the official instructions, on vanilla Ubuntu 18.04 (to ensure python3.6)
Download and unzip my sample pyproject.toml-based python package myproj.zip
cd into that package, change the path to habitat-sim specified in pyprojec.toml accordingly and poetry build.

Expected behavior

The above will fail with an assertion error. That shouldn't happen since after introspection, poetry will create a virtualenv with the right python version for the code to run in, so the python requirement will eventually be valid.

Note that you can also repro the above behavior without poetry and only python3.6 -m seutp from inside habitat-sim's directory.

Suggested Fix

I don't see a good reason for this check and assertion error to be in place. I would remove it altogether.

There is nothing >=py3.7 specific when parsing the setup.py file and setuptools and pip itself is able to enforce that the right python version will be used just by specifying python-requires = ... inside setup().

System Info

OS (e.g., Linux): Ubuntu 18.04
Python version: 3.6.9 to parse the setup.py, intending to use >=py3.7 to execute it.

Thanks.

aclegg3 commented 2 years ago

Interesting, thanks for the suggestion. I think this assertion exists to make the issue more obvious for local installations from source. Otherwise the errors produced by using 3.6 may be hard to parse and track back to a simple python version mismatch.

@Skylion007 any thoughts on this?

Skylion007 commented 2 years ago

@aclegg3 Yeah, I would remove the assertion and use the proper setup.py requires. I wonder if there is a way to specify the python version using PEP517.

Skylion007 commented 2 years ago

@bergercookie Would gladly look at a PR that fixes our setup.py if you have a suggestion? Alternatively, would listing the version properly via setup.cfg or pyproject solve the issue?

facebookresearch / habitat-sim