Deprecate setup_py() in favour of python_distribution

[kaos]

As commented by @jsirois, the setup_py() construct no longer fulfills its purpose as a standalone construct.

#11808 (comment)

Really, provides=setup_py() no longer makes any sense at all. Previously, you could add this to a python_library. In that case, it made sense, you were singling out a python_library for publication and needed to add publication data. Now that there is python_distribution - which already says "publish me" right on the tin, provides=setup_py(...) should probably go and all data should migrate to the target's direct fields.

This ticket is for deprecating setup_py() with new fields directly on python_distribution() instead.

[kaos]

How would this work, technically?

As setup_py() (or, python_artifact() which I'd actually prefer to use, from now on) accepts any fields on it (**kwargs) but the target python_distribution() is not as lenient to unknown fields..

Do we add all possible fields from setup(), or a catch all for fields "we do not care about", like python_distribution(kwargs={"some_other":"value"})?

Adding all fields supported by setup() feels a bit.. like a maintenance burden and unnecessary overhead. Any other ideas?

[Eric-Arellano]

I was wondering the exact same thing. I agree that adding every additional field is unmaintainable. I think an extra_kwargs field makes sense. When we have fields like entry_points that warrant extra love, we can promote them to first-class: it also wouldn't be an error to keep in extra_kwargs, just won't benefit from the special handling.

The setup_py() plugin hook would be replaced by the much more powerful proposed target generation, which would allow you to set any of the fields for the target, not just provides=.

But then we also hit an awkward part of the Target API: the fields need to be hashable, which is why things like DictField use FrozenDict. That won't flow with arbitrary types, we won't be able to coerce lists into tuples and so on because we don't know what data to expect. But, I think we can override __hash__ for the extra_kwargs field at least..I mean, the provides field somehow is hashable.

[kaos]

But I get the nagging feeling that we simply rename stuff, that way... hmm.. but maybe not so bad after all, if provides would become extra_kwargs or additional_kwargs, and the PythonArtifact type, would be a more generic Dict type that doesn't have any requirements at all, from a users perspective (and from ours, that it is hashable), that would make it so get rid of duplication, and move all required information to top level fields, which is a win.

[kaos]

On a second thought on the field name for those extra args.. what it would look like when used, I think simply setup might fit well..

python_distribution(
  name="my-distro",
  version="1.2.4",
  setup={
    "classifiers": "Tooling :: CI/CD :: Development",
  }
)

# vs
python_distribution(
  name="my-distro",
  version="1.2.4",
  extra_kwargs={
    "classifiers": "Tooling :: CI/CD :: Development",
  }
)

perhaps with a check on what fields are used for setup, so it isn't one that has a top-level field already, as that wouldn't really make any sense (either it was a duplicated value, or simply missing from the python_distributions field)

[Eric-Arellano]

Or maybe extra_setup_kwargs?

I agree this is a big improvement over having this weird, underdocumented python_artifact thing.

[Eric-Arellano]

This issue is still a good idea I think, but see #14832 as a proposal to make objects less hacky.

still a good idea I think,

I think it's a good idea because right now we are polluting the global namespace with something specific to only python_distribution.

--

I think the only blocker to this PR is figuring out how to __hash__. Could we do something like call str() on it? 👀

But then we also hit an awkward part of the Target API: the fields need to be hashable, which is why things like DictField use FrozenDict. That won't flow with arbitrary types, we won't be able to coerce lists into tuples and so on because we don't know what data to expect. But, I think we can override hash for the extra_kwargs field at least..I mean, the provides field somehow is hashable.

[cognifloyd]

Personally, I would love to see setup.py just disappear; i don't want a generated setup.py either.

With my anti-setup.py perspective, I would not want to see setup= as a kwarg on python_distribution.

pyproject.toml has a [project] section (defined by PEP 621 and https://packaging.python.org/en/latest/specifications/declaring-project-metadata/) that covers much of the metadata that setup.py normally defines. I think Setuptools is (one of?) the only backend that uses that standardized metadata.

So, I see two directions we could take to use this:

• add provides=python_project() instead of provides=python_artifact(..), that makes pants pull the details from pyproject.toml instead inlining in the BUILD file.
• pants could generate a pyproject.toml (or extend an existing one) with a [project] section based on the kwargs of python_artifact or one of the other recommendations in this issue.

Or maybe some combo of modifying the pyproject.toml to add info specified in the BUILD metadata (like dependencies), and sourcing other metadata from the in-worktree copy (like the package name and version).