Skip to content

Clean-up of setup.py#65

Merged
BenjaminRodenberg merged 6 commits intodevelopfrom
i_64
Nov 14, 2020
Merged

Clean-up of setup.py#65
BenjaminRodenberg merged 6 commits intodevelopfrom
i_64

Conversation

@BenjaminRodenberg
Copy link
Contributor

closes #64

@BenjaminRodenberg BenjaminRodenberg self-assigned this Oct 30, 2020
@BenjaminRodenberg
Copy link
Contributor Author

Result is not as nice as hoped for in #64 (comment), but still a step into the right direction. I will now add the suggested changes from #64 (comment)

@BenjaminRodenberg
Copy link
Contributor Author

If I try to replace from distutils.command.build import build with from setuptools.command.build_py import build_py as build, I get an error for python3 setup.py test and python3 setup.py install --user:

~/python-bindings$ python3 setup.py test
running test
WARNING: Testing via this command is deprecated and will be removed in a future version. Users looking for a generic test entry point independent of test runner are encouraged to use tox.
running egg_info
creating pyprecice.egg-info
writing pyprecice.egg-info/PKG-INFO
writing dependency_links to pyprecice.egg-info/dependency_links.txt
writing requirements to pyprecice.egg-info/requires.txt
writing top-level names to pyprecice.egg-info/top_level.txt
writing manifest file 'pyprecice.egg-info/SOURCES.txt'
reading manifest file 'pyprecice.egg-info/SOURCES.txt'
reading manifest template 'MANIFEST.in'
warning: no files found matching 'test/*.pxd'
Traceback (most recent call last):
  File "setup.py", line 105, in <module>
    setup(
  File "/usr/lib/python3/dist-packages/setuptools/__init__.py", line 144, in setup
    return distutils.core.setup(**attrs)
  File "/usr/lib/python3.8/distutils/core.py", line 148, in setup
    dist.run_commands()
  File "/usr/lib/python3.8/distutils/dist.py", line 966, in run_commands
    self.run_command(cmd)
  File "/usr/lib/python3.8/distutils/dist.py", line 985, in run_command
    cmd_obj.run()
  File "/usr/lib/python3/dist-packages/setuptools/command/test.py", line 237, in run
    with self.project_on_sys_path():
  File "/usr/lib/python3.8/contextlib.py", line 113, in __enter__
    return next(self.gen)
  File "/usr/lib/python3/dist-packages/setuptools/command/test.py", line 152, in project_on_sys_path
    self.run_command('egg_info')
  File "/usr/lib/python3.8/distutils/cmd.py", line 313, in run_command
    self.distribution.run_command(command)
  File "/usr/lib/python3.8/distutils/dist.py", line 985, in run_command
    cmd_obj.run()
  File "/usr/lib/python3/dist-packages/setuptools/command/egg_info.py", line 297, in run
    self.find_sources()
  File "/usr/lib/python3/dist-packages/setuptools/command/egg_info.py", line 304, in find_sources
    mm.run()
  File "/usr/lib/python3/dist-packages/setuptools/command/egg_info.py", line 538, in run
    self.prune_file_list()
  File "/usr/lib/python3/dist-packages/setuptools/command/egg_info.py", line 590, in prune_file_list
    build = self.get_finalized_command('build')
  File "/usr/lib/python3.8/distutils/cmd.py", line 299, in get_finalized_command
    cmd_obj.ensure_finalized()
  File "/usr/lib/python3.8/distutils/cmd.py", line 107, in ensure_finalized
    self.finalize_options()
  File "setup.py", line 91, in finalize_options
    super().finalize_options()
  File "/usr/lib/python3/dist-packages/setuptools/command/build_py.py", line 34, in finalize_options
    orig.build_py.finalize_options(self)
  File "/usr/lib/python3.8/distutils/command/build_py.py", line 43, in finalize_options
    self.set_undefined_options('build',
  File "/usr/lib/python3.8/distutils/cmd.py", line 287, in set_undefined_options
    src_cmd_obj.ensure_finalized()
  File "/usr/lib/python3.8/distutils/cmd.py", line 107, in ensure_finalized
    self.finalize_options()
  File "setup.py", line 91, in finalize_options
    super().finalize_options()

...

  File "/usr/lib/python3/dist-packages/setuptools/command/build_py.py", line 34, in finalize_options
    orig.build_py.finalize_options(self)
  File "/usr/lib/python3.8/distutils/command/build_py.py", line 43, in finalize_options
    self.set_undefined_options('build',
  File "/usr/lib/python3.8/distutils/cmd.py", line 287, in set_undefined_options
    src_cmd_obj.ensure_finalized()
  File "/usr/lib/python3.8/distutils/cmd.py", line 107, in ensure_finalized
    self.finalize_options()
  File "setup.py", line 91, in finalize_options
    super().finalize_options()
  File "/usr/lib/python3/dist-packages/setuptools/command/build_py.py", line 34, in finalize_options
    orig.build_py.finalize_options(self)
  File "/usr/lib/python3.8/distutils/command/build_py.py", line 43, in finalize_options
    self.set_undefined_options('build',
  File "/usr/lib/python3.8/distutils/cmd.py", line 287, in set_undefined_options
    src_cmd_obj.ensure_finalized()
  File "/usr/lib/python3.8/distutils/cmd.py", line 107, in ensure_finalized
    self.finalize_options()
  File "setup.py", line 91, in finalize_options
    super().finalize_options()
  File "/usr/lib/python3/dist-packages/setuptools/command/build_py.py", line 34, in finalize_options
    orig.build_py.finalize_options(self)
  File "/usr/lib/python3.8/distutils/command/build_py.py", line 43, in finalize_options
    self.set_undefined_options('build',
  File "/usr/lib/python3.8/distutils/cmd.py", line 286, in set_undefined_options
    src_cmd_obj = self.distribution.get_command_obj(src_cmd)
  File "/usr/lib/python3.8/distutils/dist.py", line 851, in get_command_obj
    cmd_obj = self.command_obj.get(command)
RecursionError: maximum recursion depth exceeded while calling a Python object

@BenjaminRodenberg BenjaminRodenberg changed the title Clean-up setup.py Clean-up of setup.py Nov 2, 2020
@BenjaminRodenberg
Copy link
Contributor Author

I have the impression that my_build is actually also not needed. This needs some further testing, but if we can get rid of my_build all the connected problems are also gone and setup.py finally becomes sane.

@ajaust
Copy link
Contributor

ajaust commented Nov 2, 2020

It does not work completely. When I try to install the bindings using the standard installation options of Spack (by hand) via
python3 setup.py --no-user-cfg install --prefix=/tmp/py-pyprecice --single-version-externally-managed --root=/
I get the target directory, but no shared libraries are copies.

When adaptiing the setup.py to contain a line ext_modules like here

setup(
    name=APPNAME,
    version=str(APPVERSION),
    description='Python language bindings for the preCICE coupling library',
    long_description=long_description,
    long_description_content_type='text/markdown',
    url='https://github.com/precice/python-bindings',
    author='the preCICE developers',
    author_email='info@precice.org',
    license='LGPL-3.0',
    python_requires='>=3',
    install_requires=['numpy', 'mpi4py'],  # mpi4py is only needed, if preCICE was compiled with MPI, see https://github.com/precice/python-bindings/issues/8
    cmdclass={'test': my_test,
              'build_ext': my_build_ext},
    package_data={ 'precice': ['*.pxd']},
    include_package_data=True,
    ext_modules = get_extensions(False),
    zip_safe=False  #needed because setuptools are used
)

the shared libraries are copied. However, it ALSO copies the libraries for the test. I am not sure if that should be like that.

Note: I have not tested if the resulting python module works.

Copy link
Member

@IshaanDesai IshaanDesai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Current state works well with the fenics-adapter and a prototype tutorial case.

Copy link
Member

@IshaanDesai IshaanDesai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Installation works. Checked with the fenics-adapter and a tutorial case.
I did not check the Spack package usage.

@BenjaminRodenberg
Copy link
Contributor Author

I tried to add some print statements for debugging in precice.pyx, but rebuilding was not triggered (tried using pip3 install --user . and python3 setup.py install --user). From my point of view this shows that the new setup.py still has some flaws.

This reverts commit abf1396.
@BenjaminRodenberg
Copy link
Contributor Author

I tried to add some print statements for debugging in precice.pyx, but rebuilding was not triggered (tried using pip3 install --user . and python3 setup.py install --user). From my point of view this shows that the new setup.py still has some flaws.

Reverted abf1396 via 477526a. Now everything works, but we - again - depend on distutils.

@BenjaminRodenberg
Copy link
Contributor Author

BenjaminRodenberg commented Nov 11, 2020

f82d5e4 allows us to get rid of the distutils dependency. From my point of view the most important use-cases are supported:

  • python3 setup.py install --user builds (if necessary) and installs the package
  • python3 setup.py build_ext builds the package
  • python3 setup.py test builds the mock package and runs the tests
  • pip3 install --user . builds (if necessary) and installs the package

However, there is one downside:

  • python3 setup.py build does not do anything. I tried raising an exception, if build is called (i.e. build is not supported. Please use build_ext), but this lead to trouble with the install command.

I think this is still not a perfect solution, but as far as I am concerned it looks to me at least like an improvement. Any opinions?

@BenjaminRodenberg BenjaminRodenberg marked this pull request as ready for review November 11, 2020 18:12
Copy link
Contributor Author

@BenjaminRodenberg BenjaminRodenberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To summarize the current state:

  • custom commands only override finalize_options. Not initialize_options.
  • my_build is not used anymore.
  • my_install inherits from setuptools.commands.install, not distutils.commands.install.
  • install option --single-version-externally-managed is available. #64

from setuptools import setup
from setuptools import Command
from setuptools.command.test import test
from setuptools.command.install import install
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized that this version now uses setuptools.command.install and not distutils.command.install. This makes --single-version-externally-managed is available (python3 setup.py install --help). See #64.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I will check if it works with the standard installation procedure of Spack now.

@ajaust
Copy link
Contributor

ajaust commented Nov 14, 2020

I was able to install the bindings from this PR (branch i_64) with the following (hacked) Spack package.py:

# Copyright 2013-2019 Lawrence Livermore National Security, LLC and other
# Spack Project Developers. See the top-level COPYRIGHT file for details.
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)

from spack import *


class PyPyprecice(PythonPackage):
    """
    This package provides python language bindings for the
    C++ library preCICE.
    """

    homepage = "https://www.precice.org"
    git = "https://github.com/precice/python-bindings.git"
    url = "https://github.com/precice/python-bindings/archive/v2.0.0.1.tar.gz"
    maintainers = ["ajaust", "BenjaminRueth"]

    # Always prefer final version of release candidate
    version("develop", branch="i_64")
    version("2.1.1.1", sha256="972f574549344b6155a8dd415b6d82512e00fa154ca25ae7e36b68d4d2ed2cf4")
    version("2.1.0.1", sha256="ac5cb7412c6b96b08a04fa86ea38e52d91ea739a3bd1c209baa93a8275e4e01a")
    version("2.0.2.1", sha256="c6fca26332316de041f559aecbf23122a85d6348baa5d3252be4ddcd5e94c09a")
    version("2.0.1.1", sha256="2791e7c7e2b04bc918f09f3dfca2d3371e6f8cbb7e57c82bd674703f4fa00be7")
    version("2.0.0.2", sha256="5f055d809d65ec2e81f4d001812a250f50418de59990b47d6bcb12b88da5f5d7")
    version("2.0.0.1", sha256="96eafdf421ec61ad6fcf0ab1d3cf210831a815272984c470b2aea57d4d0c9e0e")

    # Import module as a test
    import_modules = ["precice"]

#    patch("deactivate-version-check-via-pip.patch")
    patch("remove-packaging-and-pip.patch")

    variant("mpi", default=True, description="Enables MPI support")

    depends_on("mpi", when="+mpi")
    depends_on("precice@2.1.1")
    depends_on("precice@2.1.1", when="@2.1.1.1:2.1.1.99")
    depends_on("precice@2.1.0", when="@2.1.0.1:2.1.0.99")
    depends_on("precice@2.0.2", when="@2.0.2.1:2.0.2.99")
    depends_on("precice@2.0.1", when="@2.0.1.1:2.0.1.99")
    depends_on("precice@2.0.0", when="@2.0.0.1:2.0.0.99")

    depends_on("python@3:", type=("build", "run"))
    depends_on("py-setuptools", type="build")
    depends_on("py-wheel", type="build")
    depends_on("py-numpy", type=("build", "run"))
    depends_on("py-mpi4py", type=("build", "run"), when="+mpi")
    depends_on("py-cython@0.29:", type=("build"))

    phases = ['build_ext', 'install']

    def build_ext_args(self, spec, prefix):
        return [
            "--include-dirs=" + spec["precice"].headers.directories[0],
            "--library-dirs=" + spec["precice"].libs.directories[0]
        ]
    #def install(self, spec, prefix):
        #self.setup_py("install", "--prefix={0}".format(prefix))

I needed a new patch:

diff --git a/setup.py b/setup.py
index b31a28e..daa5b47 100644
--- a/setup.py
+++ b/setup.py
@@ -1,15 +1,6 @@
 import os
 import warnings
-from packaging import version
-import pip
-
-if version.parse(pip.__version__) < version.parse("19.0"):
-    # version 19.0 is required, since we are using pyproject.toml for definition of build-time depdendencies. See https://pip.pypa.io/en/stable/news/#id209
-    warnings.warn("You are using pip version {}. However, pip version > 19.0 is recommended. You can continue with the installation, but installation problems can occour. Please refer to https://github.com/precice/python-bindings#build-time-dependencies-cython-numpy-defined-in-pyprojecttoml-are-not-installed-automatically for help.".format(pip.__version__))
-
-if version.parse(pip.__version__) < version.parse("10.0.1"):        
-    warnings.warn("You are using pip version {}. However, pip version > 10.0.1 is required. If you continue with installation it is likely that you will face an error. See https://github.com/precice/python-bindings#version-of-pip3-is-too-old".format(pip.__version__))
-
+from setuptools._vendor.packaging import version
 from setuptools import setup
 from setuptools.command.test import test
 from Cython.Distutils.extension import Extension

This would need some polishing, but in general it seems to be fine. I tried to use py-packaging from Spack to provide the packaging module, but that still failed for me.

Copy link
Contributor

@ajaust ajaust left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look good and changes to be made to the Spack package are mentioned above, see #65 (comment).

@BenjaminRodenberg BenjaminRodenberg merged commit a51a0e1 into develop Nov 14, 2020
@BenjaminRodenberg BenjaminRodenberg deleted the i_64 branch November 14, 2020 19:27
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

Successfully merging this pull request may close these issues.

setup.py does not provide option --single-version-externally-managed

3 participants