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

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.

About this issue

Original URL
State: open
Created 3 years ago
Comments: 17 (13 by maintainers)

Most upvoted comments

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.

jsirois on Aug 15, 2021

@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

dtmistry on Feb 18, 2022

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.

benjyw on Aug 16, 2021

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

dtmistry on Aug 16, 2021

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 on Aug 15, 2021