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).