Don't include extra dependencies as required in generated setup.py

[dtmistry]

I've a repo that publishes multiple namespaced packages. A package could depend on another package in the same repo.

For e.g, the auth pkg depends on the core pkg and some other 3rd-party libs.

When executing ./pants package /path/to/auth/dist/target with the below python_distribution target -

python_distribution(
  name="dist",
  dependencies=[
    ":auth"
  ],
  provides=setup_py(
    name="auth",
    extras_requires={
      "auth_saml": ["python3-saml==1.7.0"],
      "auth_okta": [ "okta==0.0.4"]
    }
  ),
  setup_py_commands=["bdist_wheel", "sdist"]
)

Pants correctly includes the extras_requires in the generated setup.py. But, it also includes those dependencies in the install_requires.

There should be way to tell Pants to not include these extra dependencies in the generated install_requires when running the package goal.

[benjyw]

Sorry for the delay, looking at this now.

[benjyw]

Pants sets install_requires based on the actual requirements of the code, deduced from implicit and explicit dependencies. So, just to be sure I understand the issue - you're saying you'd like it not to do that for some deps? Or that it's incorrectly detecting something as a requirement (based on inferring from imports) when it shouldn't?

[jsirois]

I think @dtmistry must mean Pants is creating incorrect dists.

Leaving Pants aside - if I hand-author a dist that has conditional functionality and that functionality has additional deps beyond the core code, I'll probably structure that dist with extras to pull in those additional deps only when asked for by users of the dist (e.g.: "requests[security]" vs "requests"). In this example, Pants forces the dependencies of the "security" extra on every consumer of the dist with no option since it infers deps but does not infer context (like a try: import ... except ImportError: conditional dep).

[jsirois]

It seems like Pants needs to treat extras as a 1st class concept in python_distribution fields.

Perhaps like so:

python_distribution(
  name="dist",
  dependencies=[
    ":auth"
  ],
  extras={
      "auth_saml": ["python3-saml"],
      "auth_okta": ["okta"]
  },
  provides=setup_py(
    name="auth",
  ),
  setup_py_commands=["bdist_wheel", "sdist"]
)

The big change from the extras_requires setup kwarg is that the lists of requirements now should just be requirement names. Pants should calculate the version. And since the extras field is now 1st class, Pants can do the dependency subtraction from install_requires when it generates setup.py.

[dtmistry]

Pants sets install_requires based on the actual requirements of the code, deduced from implicit and explicit dependencies. So, just to be sure I understand the issue - you're saying you'd like it not to do that for some deps? Or that it's incorrectly detecting something as a requirement (based on inferring from imports) when it shouldn't?

It's not incorrectly detecting a requirement. Like John mentioned in this comment, Pants should subtract the extra_requires ( or the proposed extras ) from the install_requires in the generated setup.py

[benjyw]

Got it. But these are real imports, so I guess the code is handling import errors if the extra is not present?

[Eric-Arellano]

But these are real imports, so I guess the code is handling import errors if the extra is not present?But these are real imports, so I guess the code is handling import errors if the extra is not present?

Yeah, that's up to the distribution author and not something Pants needs to be worried about imo. Each plugin has differently functionality, but often I've seen the try except mentioned by John. For example, remove the [colors] extra from isort and you get:

12:43:28.24 [WARN] Completed: lint - isort failed (exit code 1).

Sorry, but to use --color (color_output) the colorama python package is required.

Reference: https://pypi.org/project/colorama/

You can either install it separately on your system or as the colors extra for isort. Ex:

$ pip install isort[colors]

[benjyw]

But these are real imports, so I guess the code is handling import errors if the extra is not present?But these are real imports, so I guess the code is handling import errors if the extra is not present?

Yeah, that's up to the distribution author and not something Pants needs to be worried about imo.

I'm trying to find out what @dtmistry's actual case is here.

[dtmistry]

I'm trying to find out what @dtmistry's actual case is here.

Sorry for the very delayed reply.

Yes the package will handle import errors resulting from optional dependencies. But as of now, there is no way to tell Pants about those optional dependencies in the python_distribution target.

[benjyw]

OK, so it sounds like we need first-class support for extras, as @jsirois mentioned.

[benjyw]

@dtmistry do you want to take a stab at implementing this? We can give guidance.

[kaos]

+1. I've run across this as well.

[dtmistry]

@dtmistry do you want to take a stab at implementing this? We can give guidance.

Sure. I'll start by getting familiar with the code. And I'll reach out in slack with questions before implementing any changes

[alexeckert]

Have a similar issue, where I want to use extras and import packages optionally in a base library if they are present and not bake them directly into the base library (as they would be auto inferred, even though they should be optional and dynamically imported).

[benjyw]

Good to know @alexeckert. I assume @dtmistry found another solution, but we welcome contributions and can guide on this.

[cognifloyd]

I just ran into this as well.

In my code there is a try/except block around an import for a 3rd party dep so that it is an optional dependency. I want to add a python_requirement for that target instead of adding it to a requirements.txt file. I think defining such extras close to the code that uses it would be ideal.

And, like others here, I also want the optional dep to be included in the lockfile and in extras_require (but not in install_requires) in the generated setup.py.

Side note: pypa changed the name from extras_require in setup.py and setup.cfg to optional-dependencies when defining the deps in pyproject.toml. So, we might (or might not) want to call this new feature "extras" in pants.

One idea: Instead of defining the extra on the python_distribution, mark the python_requirement target so that it becomes an extra. Or, instead of doing that, use an address to the indicated dep.

So, modifying John's example, maybe we could do this to mark a dep in a root requirements.txt file as optional.

python_distribution(
    ...
    optional_dependencies={
        "auth_saml": ["//reqs#python3-saml"],
        "auth_okta": ["//reqs#okta"]
    },
    ...
)

Or if someone wanted to keep a separate target for the optional deps, they could do something like:

python_requirements(name="optional_requirements", ...)
python_requirement(name="okta_requirement", requirement="...")
python_distribution(
    ...
    optional_dependencies={
        "auth_saml": [":optional_requirements"],
        "auth_okta": [":okta_requirement"],
    },
    ...
)

[gauthamnair]

Have we considered adding the metadata closer to the source? Its the python source that knows what it can optionally depend on. Suppose we weren't distributing the library, but using it internally in the monorepo, but with optional dependencies. Some monorepo consumers use it with certain requirements, others with other requirements or dependencies. Do we use parametrize currently to model that?

# a new target
named_dependency_group(
   name="auth_saml",
   dependencies=["//reqs#python3-saml"]
)

named_dependency_group(
   name="auth_okta",
   dependencies=["//reqs#okta"]
)

python_source(
    name="lib_entrypoint",
    source="lib.py",
    dependencies=[...],  # make sure to annotate the .py so that it does not infer deps on these, 
    optional_dependencies=[":auth_saml", ":auth_okta"]
)

If we annotate close to the source, then the distribution can be set up to infer the optional_dependencies from the sources consumed. And internal consumers could add the named dependency groups when they consume lib_entrypoint as explicit deps.

[github-actions[bot]]

This issue has been open for over one year without activity and is not labeled as a bug. It has been labeled as stale to invite any further updates. If you can confirm whether the issue is still applicable to the latest version of Pants, appears in other contexts, or its priority has changed, please let us know. Please feel free to close this issue if it is no longer relevant.