port setuptools options to declarative config

[eli-schwartz]

No description provided.

[FelixSchwarz]

LGTM

some potential follow-up improvements (not sure what @hodgestar thinks about these):

• Do we need to list the "tests" packages? I think this ensures they are present in the final wheel which is not ideal (though they should be present in sdist!).
• How about also referencing the license text? (license_file = COPYING)

[eli-schwartz]

I thought about adding license_files = COPYING, but it's a bit extraneous. bdist_wheel already defaults to

if 'license_file' not in metadata and 'license_files' not in metadata:
    patterns = ('LICEN[CS]E*', 'COPYING*', 'NOTICE*', 'AUTHORS*')

However, now that I check again, it is only done for wheel, not for sdist, and the real reason COPYING is even in the sdist is due to MANIFEST.in, so... we can move it from MANIFEST.in to setup.cfg

[eli-schwartz]

I agree that tests should not really be included in the final wheel, but that's a separate topic. It should be sufficient to distribute them via MANIFEST.in, but you might also want to move them to a top-level tests/ directory.

[hodgestar]

Could we put just this line back? I actually prefer if files are explicitly listed in MANIFEST.in rather than having to guess what other logic elsewhere is including files.

[hodgestar]

I am fine if it is also listed in setup.cfg -- for me that's actually separate information, i.e. "this is a file containing the license" while MANIFEST.in is "please include this file in the source distribution".

[eli-schwartz]

It's a file specifically called out in the metadata, just like py_modules or packages. But instead of declaring it as an importable source file, it's declared as a dist-info file which must be installed as wheel metadata. Since it must be installed as wheel metadata, it also must be included in the source dist in order to guarantee it makes its way into the wheel, and is thus guaranteed to be collected during the dist creation.

I disagree with your analysis -- I don't view this as guessable logic -- but if you still want it I'll put it back.

[hodgestar]

I would still like it back -- I have been bitten so often by files not being included in the source distribution that I just want one place that I can look.

[eli-schwartz]

Ok, will do.

[hodgestar]

Thank you! Apologies for being a pain about it. :|

[eli-schwartz]

lol, I wish every PR review was this simple to solve.

"Looks good, just please include this minor nitpicky line that I really like even though it's a no-op."

(I mean, it's lots better than "I disagree with the functionality and would rather you write your own separate program to do X". I've gotten that once or twice for stuff I thought was a no-brainer bugfix.)

[hodgestar]

Other than the small change I requested, looks good to me too.

I agree that moving tests into a tests/... folder would be good but is a separate topic (and should not happen in this PR :).

[FelixSchwarz]

I thought about adding license_files = COPYING, but it's a bit extraneous.

My main reason for requesting was that the license file would be present in the dist-info all the time. Also it potentially helps distro tooling which can automate python packaging better (at least in Fedora we require that the license file is declared separately so we can ensure it is always installed even if the operator chooses not to install documentation files).

[rjollos]

I don't recall if it's been discussed in this project yet, but the test command is deprecated.

[eli-schwartz]

That should really be its own issue, and actually this coincides very well with the other mention here about reorganizing into a tests/ directory.

[hodgestar]

Agreed -- let's leave messing with the test suite out of things for now.

[hodgestar]

Thanks! Will merge as soon as CI has finished running (assuming it runs successfully, although hard to see why it wouldn't).

[eli-schwartz]

It's been two days but the CI finally passed. ;)

[hodgestar]

Merged!

@eli-schwartz Thank you!