jaraco / irc

Full-featured Python IRC library for Python.
MIT License
392 stars 86 forks source link

Only require backports.unittest_mock if we're building for python 3.2 or less #77

Closed abadger closed 8 years ago

abadger commented 8 years ago

When we build on 3.3+ we shouldn't need the backports.unittest_mock library installed as the stdlib has the proper library at the proper location.

For distros that ship with python3.3+ this means they don't have to package the backports.unittest_mock package for python3.

jaraco commented 8 years ago

I received this same request in jaraco.timing #1. I still have the reservations I mentioned there. I also have the reservation that if I accept this change here, I probably need to accept it across the dozens of projects that use it.

This request and the sister one is signaling to me that there's a defect here, but I think the defect lies with setuptools and/or pytest-runner not supporting environment markers. If environment markers were supported in tests_require, this platform selection could be made earlier, on backports.unittest_mock, but still be declarative. Given the state of the art, however, I lean toward always requiring backports.unittest_mock.

I'm willing to be persuaded otherwise. Given the concerns I outline above, what's your take? What is the impact of requiring the backport even on Python 3.3+?

abadger commented 8 years ago

I feel the same way about environment markers. When I first started work on this patch I wanted to use them but quickly found that they weren't supported in the field we needed.

I think keeping dependencies low is a pretty important goal for making something appealing to certain segments of the userbase (distro packagers who look at how much additional packaging work they'll need to do to make use of the package that they really want, embedded systems developers who are looking at space on a limited storage device, people whose workplaces demand auditing of all their third-party code, etc) but it's not necessarily a goal that is important for someone who can pip install packages at will.

For this package, I think the people who are impacted by a larger dependency set could be mollified with a comment in the setup.py saying that a dependency can be removed under certain conditions. I'll probably continue to use this patch to do exactly that removal in the Fedora package that I maintain even if the change doesn't make it upstream. It doesn't affect space or functionality in the built packages and the way you've implemented backports.unittest_mock means that the patch is simple to keep working with newer versions of the dependent packages so that's not a big deal. If there's a comment added to the setup.py then other integrators will also be able to tell that they can cut their dependencies here.

In a more general discussion of keeping pure declarative style vs being minimal with deps, I suppose there's two things to think about.

jaraco commented 8 years ago

Thanks for the detailed description. I'm glad you're able to use the workaround downstream. I think that's the best place for the patch right now. I'd gladly accept a PR to signal the safety of such an operation in a comment. And I'll use this use case and others to work toward a packaging solution that's less invasive (i.e. markers on test requirements).