Repository: takluyver/flit (python)

Issue:
Support for extras_require
I was trying to port gidgethub to Flit, but  then I realized I had some `extra_requires` entries:

https://github.com/brettcannon/gidgethub/blob/576a64f57fccba85e6060350491e33a8d0185ac6/setup.py#L37-L45

Is there any support for this sort of stuff in Flit that the docs don't cover? If not, any thoughts on adding support?

Pull request description:
Implement extras_require
Fixes #144

Patch:
diff --git a/README.rst b/README.rst
index 43927a1..b758bd1 100644
--- a/README.rst
+++ b/README.rst
@@ -75,7 +75,7 @@ To install the development version of Flit from Github::
 
     git clone https://github.com/takluyver/flit.git
     cd flit
-    python3 -m pip install docutils requests pytoml
+    python3 -m pip install docutils requests
     python3 -m flit install
 
 You may want to use the ``--symlink`` or ``--pth-file`` options so you can test
diff --git a/doc/cmdline.rst b/doc/cmdline.rst
index a6650d0..d4b79c2 100644
--- a/doc/cmdline.rst
+++ b/doc/cmdline.rst
@@ -2,7 +2,7 @@ Flit command line interface
 ===========================
 
 All operations use the ``flit`` command, followed by one of a number of
-subcommands.
+subcomamnds.
 
 Common options
 --------------
@@ -82,7 +82,14 @@ Install the package on your system.
 .. option:: --deps <dependency option>
 
    Which dependencies to install. One of ``all``, ``production``, ``develop``,
-   or ``none``. Default ``all``.
+   or ``none``. ``all`` and ``develop`` install the extras ``test``, ``docs``,
+   and ``dev``. Default ``all``.
+
+.. option:: --extras <extra[,extra,...]>
+
+   Which named extra features to install dependencies for. Specify ``all`` to
+   install all optional dependencies, or a comma-separated list of extras.
+   Default depends on ``--deps``.
 
 .. option:: --user
 
diff --git a/doc/pyproject_toml.rst b/doc/pyproject_toml.rst
index 2fa18ed..f727af7 100644
--- a/doc/pyproject_toml.rst
+++ b/doc/pyproject_toml.rst
@@ -67,12 +67,18 @@ requires
             "configparser; python_version == '2.7'"
       ]
 
-dev-requires
-  Packages that are required for development. This field is in the same format
-  as ``requires``.
+requires-extra
+  Lists of packages needed for every optional feature. The requirements
+  are specified in the same format as for ``requires``. The requirements of
+  the two reserved extras ``test`` and ``doc`` as well as the extra ``dev``
+  are installed by ``flit install``. For example:
+
+  .. code-block:: toml
+
+      [tool.flit.metadata.requires-extra]
+      test = ["pytest>=2.7.3", "pytest-cov"]
+      doc = ["sphinx"]
 
-  These are not (yet) encoded in the wheel, but are used when doing
-  ``flit install``.
 description-file
   A path (relative to the .toml file) to a file containing a longer description
   of your package to show on PyPI. This should be written in `reStructuredText
diff --git a/flit/__init__.py b/flit/__init__.py
index c3535a1..e4554ff 100644
--- a/flit/__init__.py
+++ b/flit/__init__.py
@@ -60,7 +60,12 @@ def main(argv=None):
     )
     add_shared_install_options(parser_install)
     parser_install.add_argument('--deps', choices=['all', 'production', 'develop', 'none'], default='all',
-        help="Which set of dependencies to install")
+        help="Which set of dependencies to install. If --deps=develop, the extras dev, doc, and test are installed"
+    )
+    parser_install.add_argument('--extras', default=(), type=lambda l: l.split(',') if l else (),
+        help="Install the dependencies of these (comma separated) extras additionally to the ones implied by --deps. "
+             "--extras=all can be useful in combination with --deps=production, --deps=none precludes using --extras"
+    )
 
     parser_installfrom = subparsers.add_parser('installfrom',
        help="Download and install a package using flit from source"
@@ -107,7 +112,8 @@ def main(argv=None):
         from .install import Installer
         try:
             Installer(args.ini_file, user=args.user, python=args.python,
-                      symlink=args.symlink, deps=args.deps, pth=args.pth_file).install()
+                      symlink=args.symlink, deps=args.deps, extras=args.extras,
+                      pth=args.pth_file).install()
         except (common.NoDocstringError, common.NoVersionError) as e:
             sys.exit(e.args[0])
     elif args.subcmd == 'installfrom':
diff --git a/flit/common.py b/flit/common.py
index b73ebc9..063a39d 100644
--- a/flit/common.py
+++ b/flit/common.py
@@ -227,20 +227,31 @@ class Metadata:
     requires_external = ()
     provides_extra = ()
 
-    metadata_version="2.1"
-
-    # this is part of metadata spec 2, we are using it for installation but it
-    # doesn't actually get written to the metadata file
-    dev_requires = ()
+    metadata_version = "2.1"
 
     def __init__(self, data):
         self.name = data.pop('name')
         self.version = data.pop('version')
         self.author_email = data.pop('author_email')
         self.summary = data.pop('summary')
+        requires_extra = data.pop('requires_extra', {})
+        dev_requires = data.pop('dev_requires', None)
+        if dev_requires is not None:
+            if 'dev' in requires_extra:
+                raise ValueError('Ambiguity: Encountered dev-requires together with its replacement requires-extra.dev.')
+            log.warning('“dev-requires = ...” is obsolete. Use “requires-extra = {"dev" = ...}” instead.')
+            requires_extra.setdefault('dev', []).extend(dev_requires)
+        explicit_extras = data.pop('provides_extra', ())
+        self.provides_extra = list(set(explicit_extras) | requires_extra.keys())
         for k, v in data.items():
             assert hasattr(self, k), "data does not have attribute '{}'".format(k)
             setattr(self, k, v)
+        if requires_extra:
+            self.requires_dist = list(self.requires_dist) + [
+                '{}; extra == "{}"'.format(d, e)
+                for e, ds in requires_extra.items()
+                for d in ds
+            ]
 
     def _normalise_name(self, n):
         return n.lower().replace('-', '_')
diff --git a/flit/inifile.py b/flit/inifile.py
index 1920bd2..5e95305 100644
--- a/flit/inifile.py
+++ b/flit/inifile.py
@@ -34,6 +34,7 @@ metadata_allowed_fields = {
     'dist-name',
     'entry-points-file',
     'description-file',
+    'requires-extra',
 } | metadata_list_fields
 
 metadata_required_fields = {
@@ -231,6 +232,16 @@ def _prep_metadata(md_sect, path):
             if not all(isinstance(a, str) for a in value):
                 raise ConfigError('Expected a list of strings for {} field'
                                     .format(key))
+        elif key == 'requires-extra':
+            if not isinstance(value, dict):
+                raise ConfigError('Expected a dict for requires-extra field, found {!r}'
+                                    .format(value))
+            if not all(isinstance(e, list) for e in value.values()):
+                raise ConfigError('Expected a dict of lists for requires-extra field')
+            for e, reqs in value.items():
+                if not all(isinstance(a, str) for a in reqs):
+                    raise ConfigError('Expected a string list for requires-extra. (extra {})'
+                                        .format(e))
         else:
             if not isinstance(value, str):
                 raise ConfigError('Expected a string for {} field, found {!r}'
diff --git a/flit/install.py b/flit/install.py
index 502c4bf..c880c57 100644
--- a/flit/install.py
+++ b/flit/install.py
@@ -84,17 +84,24 @@ class RootInstallError(Exception):
         return ("Installing packages as root is not recommended. "
             "To allow this, set FLIT_ROOT_INSTALL=1 and try again.")
 
+class DependencyError(Exception):
+    def __str__(self):
+        return 'To install dependencies for extras, you cannot set deps=none.'
+
 class Installer(object):
     def __init__(self, ini_path, user=None, python=sys.executable,
-                 symlink=False, deps='all', pth=False):
+                 symlink=False, deps='all', extras=(), pth=False):
         self.ini_path = ini_path
         self.python = python
         self.symlink = symlink
         self.pth = pth
         self.deps = deps
+        self.extras = extras
         if deps != 'none' and os.environ.get('FLIT_NO_NETWORK', ''):
             self.deps = 'none'
             log.warning('Not installing dependencies, because FLIT_NO_NETWORK is set')
+        if deps == 'none' and extras:
+            raise DependencyError()
 
         self.ini_info = inifile.read_pkg_ini(ini_path)
         self.module = common.Module(self.ini_info['module'], ini_path.parent)
@@ -184,6 +191,20 @@ class Installer(object):
             for f in files:
                 self.installed_files.append(os.path.join(dirpath, f))
 
+    @property
+    def extra_reqs(self):
+        return self.ini_info['metadata'].get('requires_extra', {})
+
+    def _extras_to_install(self):
+        extras_to_install = set(self.extras)
+        if self.deps == 'all' or 'all' in extras_to_install:
+            extras_to_install |= set(self.extra_reqs.keys())
+            # We don’t remove 'all' from the set because there might be an extra called “all”.
+        elif self.deps == 'develop':
+            extras_to_install |= {'dev', 'doc', 'test'}
+        log.info("Extras to install for deps %r: %s", self.deps, extras_to_install)
+        return extras_to_install
+
     def install_requirements(self):
         """Install requirements of a package with pip.
 
@@ -196,8 +217,9 @@ class Installer(object):
             return
         if self.deps in ('all', 'production'):
             requirements.extend(self.ini_info['metadata'].get('requires_dist', []))
-        if self.deps in ('all', 'develop'):
-            requirements.extend(self.ini_info['metadata'].get('dev_requires', []))
+
+        for extra in self._extras_to_install():
+            requirements.extend(self.extra_reqs.get(extra, []))
 
         # there aren't any requirements, so return
         if len(requirements) == 0:
@@ -311,8 +333,11 @@ class Installer(object):
 
             renamed_whl = os.path.join(td, wb.wheel_filename)
             os.rename(temp_whl, renamed_whl)
+            extras = self._extras_to_install()
+            whl_with_extras = '{}[{}]'.format(renamed_whl, ','.join(extras)) \
+                if extras else renamed_whl
 
-            cmd = [self.python, '-m', 'pip', 'install', renamed_whl]
+            cmd = [self.python, '-m', 'pip', 'install', whl_with_extras]
             if self.user:
                 cmd.append('--user')
             if self.deps == 'none':
diff --git a/pyproject.toml b/pyproject.toml
index 276ef75..41b8e91 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -25,6 +25,18 @@ classifiers=["Intended Audience :: Developers",
     "Topic :: Software Development :: Libraries :: Python Modules",
 ]
 
+[tool.flit.metadata.requires-extra]
+test = [
+	"testpath",
+	"responses",
+	"pytest>=2.7.3",
+	"pytest-warnings",
+	"pytest-cov",
+]
+doc = [
+	"pygments-github-lexers",  # TOML highlighting
+]
+
 [tool.flit.metadata.urls]
 Documentation = "https://flit.readthedocs.io/en/latest/"
 


Test patch:
diff --git a/tests/samples/dev_requires_with_empty_lines.ini b/tests/samples/dev_requires_with_empty_lines.ini
deleted file mode 100644
index 31653d2..0000000
--- a/tests/samples/dev_requires_with_empty_lines.ini
+++ /dev/null
@@ -1,7 +0,0 @@
-[metadata]
-module=module1
-author=Sir Robin
-author-email=robin@camelot.uk
-home-page=http://github.com/sirrobin/module1
-dev-requires=
-	foo
diff --git a/tests/samples/extras-dev-conflict.toml b/tests/samples/extras-dev-conflict.toml
new file mode 100644
index 0000000..0fe249d
--- /dev/null
+++ b/tests/samples/extras-dev-conflict.toml
@@ -0,0 +1,13 @@
+[build-system]
+requires = ["flit"]
+
+[tool.flit.metadata]
+module = "module1"
+author = "Sir Robin"
+author-email = "robin@camelot.uk"
+home-page = "http://github.com/sirrobin/module1"
+description-file = "EG_README.rst"
+dev-requires = ["apackage"]
+
+[tool.flit.metadata.requires-extra]
+dev = ["anotherpackage"]
diff --git a/tests/samples/extras.toml b/tests/samples/extras.toml
new file mode 100644
index 0000000..16d95a5
--- /dev/null
+++ b/tests/samples/extras.toml
@@ -0,0 +1,14 @@
+[build-system]
+requires = ["flit"]
+
+[tool.flit.metadata]
+module = "module1"
+author = "Sir Robin"
+author-email = "robin@camelot.uk"
+home-page = "http://github.com/sirrobin/module1"
+description-file = "EG_README.rst"
+requires = ["toml"]
+
+[tool.flit.metadata.requires-extra]
+test = ["pytest"]
+custom = ["requests"]
diff --git a/tests/test_common.py b/tests/test_common.py
index 9b6b70d..e78355c 100644
--- a/tests/test_common.py
+++ b/tests/test_common.py
@@ -1,4 +1,3 @@
-import os
 from pathlib import Path
 from unittest import TestCase
 import pytest
@@ -7,18 +6,18 @@ from flit.common import (Module, get_info_from_module, InvalidVersion, NoVersion
      check_version, normalize_file_permissions
 )
 
-samples_dir = os.path.join(os.path.dirname(__file__), 'samples')
+samples_dir = Path(__file__).parent / 'samples'
 
 class ModuleTests(TestCase):
     def test_package_importable(self):
         i = Module('package1', samples_dir)
-        assert i.path == Path(samples_dir, 'package1')
-        assert i.file == Path(samples_dir, 'package1', '__init__.py')
+        assert i.path == samples_dir / 'package1'
+        assert i.file == samples_dir / 'package1' / '__init__.py'
         assert i.is_package
 
     def test_module_importable(self):
         i = Module('module1', samples_dir)
-        assert i.path == Path(samples_dir, 'module1.py')
+        assert i.path == samples_dir / 'module1.py'
         assert not i.is_package
 
     def test_missing_name(self):
diff --git a/tests/test_inifile.py b/tests/test_inifile.py
index c7d3e5f..0d7bab4 100644
--- a/tests/test_inifile.py
+++ b/tests/test_inifile.py
@@ -1,11 +1,11 @@
 import logging
-import pathlib
+from pathlib import Path
 
 import pytest
 
-from flit.inifile import read_pkg_ini, ConfigError, flatten_entrypoints
+from flit.inifile import read_pkg_ini, ConfigError, flatten_entrypoints, _prep_metadata
 
-samples_dir = pathlib.Path(__file__).parent / 'samples'
+samples_dir = Path(__file__).parent / 'samples'
 
 def test_invalid_classifier():
     with pytest.raises(ConfigError):
@@ -18,13 +18,9 @@ def test_classifiers_with_space():
     """
     read_pkg_ini(samples_dir / 'classifiers_with_space.ini')
 
-@pytest.mark.parametrize(('filename', 'key', 'expected'), [
-    ('requires_with_empty_lines.ini', 'requires_dist', ['foo', 'bar']),
-    ('dev_requires_with_empty_lines.ini', 'dev_requires', ['foo']),
-])
-def test_requires_with_empty_lines(filename, key, expected):
-    ini_info = read_pkg_ini(samples_dir / filename)
-    assert ini_info['metadata'][key] == expected
+def test_requires_with_empty_lines():
+    ini_info = read_pkg_ini(samples_dir / 'requires_with_empty_lines.ini')
+    assert ini_info['metadata']['requires_dist'] == ['foo', 'bar']
 
 def test_missing_entrypoints():
     with pytest.raises(FileNotFoundError):
@@ -56,3 +52,13 @@ def test_bad_description_extension(caplog):
     assert info['metadata']['description_content_type'] is None
     assert any((r.levelno == logging.WARN and "Unknown extension" in r.msg)
                 for r in caplog.records)
+
+@pytest.mark.parametrize(('erroneous', 'match'), [
+    ({'requires-extra': None}, r'Expected a dict for requires-extra field'),
+    ({'requires-extra': dict(dev=None)}, r'Expected a dict of lists for requires-extra field'),
+    ({'requires-extra': dict(dev=[1])}, r'Expected a string list for requires-extra'),
+])
+def test_faulty_requires_extra(erroneous, match):
+    metadata = {'module': 'mymod', 'author': '', 'author-email': ''}
+    with pytest.raises(ConfigError, match=match):
+        _prep_metadata(dict(metadata, **erroneous), None)
diff --git a/tests/test_install.py b/tests/test_install.py
index 1ccee24..9c248d0 100644
--- a/tests/test_install.py
+++ b/tests/test_install.py
@@ -5,10 +5,11 @@ import tempfile
 from unittest import TestCase, SkipTest
 from unittest.mock import patch
 
+import pytest
 from testpath import assert_isfile, assert_isdir, assert_islink, MockCommand
 
 from flit import install
-from flit.install import Installer, _requires_dist_to_pip_requirement
+from flit.install import Installer, _requires_dist_to_pip_requirement, DependencyError
 
 samples_dir = pathlib.Path(__file__).parent / 'samples'
 
@@ -117,6 +118,43 @@ class InstallTests(TestCase):
         assert len(calls) == 1
         assert calls[0]['argv'][1:5] == ['-m', 'pip', 'install', '-r']
 
+    def test_extras_error(self):
+        with pytest.raises(DependencyError):
+            Installer(samples_dir / 'requires-requests.toml',
+                            user=False, deps='none', extras='dev')
+
+@pytest.mark.parametrize(('deps', 'extras', 'installed'), [
+    ('none', [], set()),
+    ('develop', [], {'pytest;'}),  # TODO: why not also normal reqs, i.e. toml?
+    ('production', [], {'toml;'}),
+    ('all', [], {'toml;', 'pytest;', 'requests;'}),
+])
+def test_install_requires_extra(deps, extras, installed):
+    it = InstallTests()
+    try:
+        it.setUp()
+        ins = Installer(samples_dir / 'extras.toml', python='mock_python',
+                        user=False, deps=deps, extras=extras)
+
+        cmd = MockCommand('mock_python')
+        get_reqs = (
+            "#!{python}\n"
+            "import sys\n"
+            "with open({recording_file!r}, 'wb') as w, open(sys.argv[-1], 'rb') as r:\n"
+            "    w.write(r.read())"
+        ).format(python=sys.executable, recording_file=cmd.recording_file)
+        cmd.content = get_reqs
+
+        with cmd as mock_py:
+            ins.install_requirements()
+        with open(mock_py.recording_file) as f:
+            str_deps = f.read()
+        deps = str_deps.split('\n') if str_deps else []
+
+        assert set(deps) == installed
+    finally:
+        it.tearDown()
+
 def test_requires_dist_to_pip_requirement():
     rd = 'pathlib2 (>=2.3); python_version == "2.7"'
     assert _requires_dist_to_pip_requirement(rd) == \
diff --git a/tests/test_metadata.py b/tests/test_metadata.py
new file mode 100644
index 0000000..51870f9
--- /dev/null
+++ b/tests/test_metadata.py
@@ -0,0 +1,30 @@
+from pathlib import Path
+
+import pytest
+
+from flit.common import Metadata
+from flit.inifile import read_pkg_ini
+
+samples_dir = Path(__file__).parent / 'samples'
+
+def test_extras():
+    info = read_pkg_ini(samples_dir / 'extras.toml')
+    assert info['metadata']['requires_extra']['test'] == ['pytest']
+    assert info['metadata']['requires_extra']['custom'] == ['requests']
+
+def test_extras_dev_conflict():
+    info = read_pkg_ini(samples_dir / 'extras-dev-conflict.toml')
+    with pytest.raises(ValueError, match=r'Ambiguity'):
+        Metadata(dict(name=info['module'], version='0.0', summary='', **info['metadata']))
+
+def test_extras_dev_warning(caplog):
+    info = read_pkg_ini(samples_dir / 'extras-dev-conflict.toml')
+    info['metadata']['requires_extra'] = {}
+    meta = Metadata(dict(name=info['module'], version='0.0', summary='', **info['metadata']))
+    assert '“dev-requires = ...” is obsolete' in caplog.text
+    assert set(meta.requires_dist) == {'apackage; extra == "dev"'}
+
+def test_extra_conditions():
+    info = read_pkg_ini(samples_dir / 'extras.toml')
+    meta = Metadata(dict(name=info['module'], version='0.0', summary='', **info['metadata']))
+    assert set(meta.requires_dist) == {'toml', 'pytest; extra == "test"', 'requests; extra == "custom"'}