pypa / packaging.python.org

Python Packaging User Guide
http://packaging.python.org
1.41k stars 886 forks source link

Stance (or discussion) on src/ directory #320

Closed ctheune closed 5 years ago

ctheune commented 7 years ago

Is there an official stance on the "src/" directory thing? I'm all for it (and @hynek seems to agree) but I haven't found any discussion or explanation about it from the PyPA. The guide doesn't mention it and the sample project doesn't use it. Even if the stance is "don't use it" (which I would disagree with) then I'd love to see this discussed in the official guide so we can refer to it when people need to make up their mind or argue it.

ctheune commented 7 years ago

Some references on the discussion that support the "pro" argument:

pfmoore commented 7 years ago

I don't think there's a consensus on the best practice, and the packaging guide isn't really intended to cover general development practices, which this is. There's no discussion on testing or documentation, for similar reasons.

theacodes commented 7 years ago

Based on the doc plan in #317, this is actually a good candidate for a discussion topic. I would be willing to accept a PR to add this topic.

pfmoore commented 7 years ago

Nice, @jonparrott - I hadn't seen that doc plan. Agreed this would make a good discussion topic. Another one I've seen come up in the past is whether to put tests in the package or in a separate test directory.

theacodes commented 7 years ago

@pfmoore for sure, I'd be happy to have that topic as well.

theacodes commented 7 years ago

@ctheune if you or @hynek are interested in contributing this topic, I would love to review and merge it. :)

pfmoore commented 7 years ago

I'd also be interested in reviewing this. I tried a layout using src just recently and found there were some things that were fiddly to get right (coverage was one), so I have a test case I could use to see how the recommendations work in practice :-)

hynek commented 7 years ago

Have you seen https://hynek.me/articles/testing-packaging/ Paul? I hoped to de-fiddle the process there...

pfmoore commented 7 years ago

@hynek Yes I have. The problem I had was not with combining coverage, but with the basic coverage output from a single tox environment. It's possible it was just lack of familiarity with running coverage under tox, but from what I recall the output had some rather badly nested directories, when I was after filenames relative to src (so mypkg/__init__.py). I backed out the implementation of coverage, so I don't have it to hand at the moment, but could reimplement it if needed.

ctheune commented 7 years ago

Thanks for picking up that topic - happy to contribute although I can't promise to be very responsive at times (busy company and small child make my FLOSS contributions a bit spotty at the moment).

I haven't run into the coverage issue before - I usually have both a tox and non-tox pytest setup and ensure my coverage is right in the basic setup. I see the need for checking coverage of Python--version-specific code paths, though.

What's the next step for contributing here?

theacodes commented 7 years ago

Distill the source content into a discussion topic and send a pull request. Or, if you prefer you can author it in Google docs and I can review and place it here once it's finalized. Happy to help give you guidance on any step.

On Tue, Jun 27, 2017, 11:07 PM Christian Theune notifications@github.com wrote:

Thanks for picking up that topic - happy to contribute although I can't promise to be very responsive at times (busy company and small child make my FLOSS contributions a bit spotty at the moment).

I haven't run into the coverage issue before - I usually have both a tox and non-tox pytest setup and ensure my coverage is right in the basic setup. I see the need for checking coverage of Python--version-specific code paths, though.

What's the next step for contributing here?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/pypa/python-packaging-user-guide/issues/320#issuecomment-311565338, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPUc6Er5_1ZDv6GnMBokdAjWC5M7Ed9ks5sIe2igaJpZM4Nyan8 .

pradyunsg commented 7 years ago

Is anyone working on this right now?

theacodes commented 7 years ago

Nope.

On Fri, Aug 25, 2017, 12:11 AM Pradyun Gedam notifications@github.com wrote:

Is anyone working on this right now?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/pypa/python-packaging-user-guide/issues/320#issuecomment-324841802, or mute the thread https://github.com/notifications/unsubscribe-auth/AAPUc439KkMZ5j43eujlOvsPxjWR3LNHks5sbnOogaJpZM4Nyan8 .

ofek commented 7 years ago

I really, really dislike this structure. It makes things a bit more complicated than necessary imo. Newcomers should be considered here.

hynek commented 7 years ago

Beginners who are more likely to make mistakes in their packaging and that would hugely benefit from a more explicit setup that protects them against broken PyPI uploads and deployments? Yes I agree, they should be very much considered over personal preferences. :)

ofek commented 7 years ago

@hynek More explicit is worse for beginners when they could rather have a simple packages=find_packages() :slightly_smiling_face:

hynek commented 7 years ago

I feel like we're moving into "explicit self is bad because you have to type more" territory.

Passing a path to find_packages takes away a lot of mystery and perceived magic how it works and I'd argue you should pass it even if it's "." .

And realistically people just copy paste it anyway so I'm having a hard time to see it as a problem at all. OTOH reading it later is helpful to understand what's going on without guessing.

theacodes commented 7 years ago

For beginners we'll likely have a new tutorial using flit as a starting point. Setuptools-style packaging would likely fall under guides, and a separate guide/discussion can be created around this topic.

hynek commented 7 years ago

I'm not sure whether pushing for flit is the right way as long as it precludes the usage of tox.

theacodes commented 7 years ago

I'm not sure whether pushing for flit is the right way as long as it precludes the usage of tox.

tox does a lot of weird stuff, but how does flit preclude using tox?

hynek commented 7 years ago

It may have changed (sadly I'm on a very bad Botswanan campsite wifi so I can't double check) but last time I checked tox needed a setup.py to work?

theacodes commented 7 years ago

I think you can turn that off or use a different install command (though the later affects deps as well).

I definitely plan to try out flit significantly before writing the experimental tutorial and gathering feedback before considering making the experimental tutorial official. So this is really useful.

Also, shameless plug that tox isn't the only option.

obestwalter commented 7 years ago

but last time I checked tox needed a setup.py to work

No.

[tox]
skipsdist = True

[...]

... and you're golden.

obestwalter commented 7 years ago

@jonparrott interesting - quite new project also - how come you thought nox is needed? What's missing from tox? I am always interested in seeing how we can improve tox and maybe join forces, if you like open an issue or write on the list.

theacodes commented 7 years ago

@obestwalter mostly just frustrated with using ini files. :)

obestwalter commented 7 years ago

That's what I thought :D - also see https://github.com/tox-dev/tox/issues/600 - sorry for hijacking this - I am quiete now ...

theacodes commented 7 years ago

@obestwalter cool. :) I think there's room for both. If you ever want to chat about it feel free to reach out. :)

pradyunsg commented 6 years ago

I'm setting a reminder for myself; if no one comes around to this by mid-October, I'll give this a shot. :)

mostly just frustrated with using ini files. :)

pyproject.toml adoption -- tool.*. ✨

pradyunsg commented 6 years ago

Turns out I didn't actually get the time to doing this. If anyone else wants to take this forward, please do.

astrojuanlu commented 6 years ago

Before asking here "what is the expected structure for a discussion document about src layouts?" I went to see the rest of the discussions for inspiration, and found that:

So I wonder if adapting @hynek or @ionelmc articles on src layouts would be too long for this?

theacodes commented 6 years ago

They likely aren't. If they are, I'm happy to work with the submitter of the PR to shorten them. The existing discussions were adapted from the older documentation so they aren't necessarily representative of a "discussion".

pradyunsg commented 6 years ago

Dumping my notes here (in case someone gets to this before I do).

Documentation and Blog Posts:

A few existing "project templates:


We don't want to recommend one format for structuring projects over another -- although I think this should be discussed after the Guide has a discussion on this (or a PR for it).

I think we should make a bulleted list of merits/demerits which we discuss, then convert that into a prose in a nice discussion format. :)

pfmoore commented 6 years ago

We don't want to recommend one format for structuring projects over another

Note that flit does intend to support a single project structure. So if this guide doesn't make a specific recommendation, we may need to add a note that your build tool of choice might not support your layout (once we get to the point where we're not recommending setuptools as the primary build tool). Conversely, if we do recommend one layout, that may be an incentive for build tools to support that layout.

See https://github.com/takluyver/flit/issues/115 for some background.

astrojuanlu commented 6 years ago

More templates:

pradyunsg commented 6 years ago

Conversely, if we do recommend one layout, that may be an incentive for build tools to support that layout.

Yep -- that's what I was thinking too but @theacodes suggested otherwise. I figured we can have a discussion on this (whether to recommend one layout or not) once we have a document listing out the merits/demerits of each.

takluyver commented 6 years ago

I'm happy to hold the discussion until we've put together a summary of the pros and cons of each, but I do think that the packaging user guide should make a clear recommendation, for two reasons:

  1. It's most helpful to new developers to find out 'this is how to package your project'. Asking people to evaluate two options and work out which one is best for their situation is extra cognitive load.
  2. There's a big community advantage in having a common structure for the source files in a repository, so that you can quickly recognise what's where in an unfamiliar project. Of course it will never be 100% uniform, but I hope for 95%.

I suspect we will find that the src/ layout has more advantages, but we have to somehow judge whether those advantages outweigh the cost of switching, given that the vast majority of existing packages probably use a non-src layout.

astrojuanlu commented 6 years ago

we have to somehow judge whether those advantages outweigh the cost of switching, given that the vast majority of existing packages probably use a non-src layout

I guess existing projects would not have to switch if their current setup works well enough for them. After all, this is intended to help new developers.

takluyver commented 6 years ago

Right, but there's still a transition cost in telling new developers to do something different from most existing projects, and from (further) splitting the ecosystem of packages.

For flit, if I do move to the src layout by default, I would push projects to switch over - with a suitable transition period and hopefully some tooling to make it easier.

On Thu, 17 May 2018, 5:51 p.m. Juan Luis Cano Rodríguez, < notifications@github.com> wrote:

we have to somehow judge whether those advantages outweigh the cost of switching, given that the vast majority of existing packages probably use a non-src layout

I guess existing projects would not have to switch if their current setup works well enough for them. After all, this is intended to help new developers.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/pypa/python-packaging-user-guide/issues/320#issuecomment-389915628, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUA9frrpIgVIvENKL_o07eKCfbMEmbqks5tzZyVgaJpZM4Nyan8 .

dstufft commented 6 years ago

I think src/ makes more sense, largely because anything else plays hell with trying to run tests against the installed version of the library instead of the version that happens to be in the current directory.

hynek commented 6 years ago

I suspect we will find that the src/ layout has more advantages, but we have to somehow judge whether those advantages outweigh the cost of switching, given that the vast majority of existing packages probably use a non-src layout.

I think this is a fair statement. There are no real pros and cons thing going on here – src is objectively better in many ways. Any argument I’ve heard against it so far was either some kind of misconception or about aesthetics (although mostly “looks like java” or “that’s how we’ve always done” which I both don’t find very valuable).

I’d argue that if you want to give beginners a layout as default, it should be the one that makes it less likely for them to shoot themselves into their foot. Urging existing code bases to move OTOH is the first step for another nightmare on /r/Python.

ofek commented 6 years ago

Why can't we recommend virtual environments or pip install -e ... for testing? That solves the whole situation and is something that should be/is done anyway.

Officially recommending src/ will cause extreme churn for current devs and spur extra questions ("why is this necessary? it's ugly, other langs don't need this") for new devs when a simple disclaimer/note would suffice (definitely do this).

No one will benefit from this long term, and it will hurt our community in the short (hopefully) term.

dstufft commented 6 years ago

No it doesn't solve it for testing, because . comes first in sys.path, and for projects that want to have a tests/ directory that is importable but isn't installed, it's not even fixable in any other way.

ofek commented 6 years ago

Sorry, but could you please elaborate on that scenario?

dstufft commented 6 years ago

So a few things, first is that by default the first item on sys.path in many contexts is ., so any time you're in a context like that, by default you're going to import from the current directory instead of the virtual environment.

In some cases, this is solvable by tracking down every place you start up Python and munging sys.path to remove that (which also means that you can't just do python and then start debugging against your installed version, since that's going to pick up your current directory, you'll need a wrapper script to munge sys.path and then call python for you).

However, one very popular case when this isn't solvable, is when you have a directory structure like:

.
├── mylib
└── tests

Where mylib gets installed, but tests do not, then you will NEED to have . on the sys.path in order to be able to import your test files from tests/, which means that you will always test against the mylib that is in the current directory.

I actually spent a good chunk of time trying to solve this exact problem on the cryptography problem without switching to using a src/ directory, and at the end of the day I couldn't find another way to resolve it. It was either our tests were going to run against the local copy not the installed copy [1], or we had to move to a src/ directory.

[1] Running your tests against an installed copy is best practice, because it's the only way you can be sure that the result of packaging up your library and then installing it is actually going to work. Things like files that get missed in the packaging can make tests pass from the current directory, but fail when installed. There are also things like build steps that can't be done in-place (compiling .so's can, but for instance if you use 2to3 then -e will not do an inplace install, and will instead copy to the build directory and do the install there).

hynek commented 6 years ago

I have multiple people who screwed up their packaging and uploaded a broken package come to me and tell me that I was right™ and that they switched now. So saying it doesn’t help anyone is at least anecdotally wrong.

It seems to be one of the things that don’t seem worth it until it is. Which is kinda exactly what beginners should be protected from because they can’t make that call themselves and are more likely to make mistakes that could be caught by it.

dstufft commented 6 years ago

It seems to be one of the things that don’t seem worth it until it is. Which is kinda exactly what beginners should be protected from because they can’t make that call themselves and are more likely to make mistakes that could be caught by it.

It's not even really just beginners (although they need it for sure). I'm far from a beginner and I didn't believe in src/ until the cryptography issue cropped up and I spent multiple days trying to make any other solution work. It's non obvious, even to experts but it solves real problems and should be the default.

hynek commented 6 years ago

It's not even really just beginners (although they need it for sure). I'm far from a beginner and I didn't believe in src/ until the cryptography issue cropped up and I spent multiple days trying to make any other solution work.

Oh 💯 same here. I switched after screwing up myself.

ctheune commented 6 years ago

I like where this is headed! 🦄 I guess I just have to keep my mouth shut now. :)

pradyunsg commented 6 years ago

The following are the points that have been made in the various blog posts I linked above:

For src:

For non-src:

None of the following discuss why not use src/ but do not use it:

As such, I'm still looking for more points in favour of non-src setups or against src setup.

pradyunsg commented 6 years ago

I'm not sure if this was the best way to spend my time but here's the first 60 projects from libraries.io's PyPI "Most Used" and the layout they're using today (on the default branch when you land on their repo). I think this is a strong heuristic for what the community does today.

Click to see a table that involved a lot of clicking to create. project | uses 'src/'? --- | --- requests | N django | N flask | N six | N jinja2 | N numpy | N markupsafe | N werkzeug | N pytz | Y psycopg2 | N gunicorn | N pytest | N pyyaml | Y sphinx | N dateutil | N mock | N coverage.py | N Pillow | Y nose | N itsdangerous | N click | N sqlalchemy | Y (they use lib/) scipy | N lxml | Y argparse | N flake8 | Y matplotlib | Y ipython | N setuptools | N pygments | N beautifulsoup | N pandas | N boto | N redis-py | N flask-sqlalchemy | N tornado | N djangorestframework | N simplejson | N certifi | N django-database-url | N markdown | N docutils | N chardet | N decorator | Y pyparsing | N pymongo | N babel | N pytest-cov | Y pycrypto | Y tox | N south | N httplib2 | Y flask-wtf | N idna | N celery | N gevent | Y scikit-learn | N wtfforms | N Notes about the contents: - six, argparse, django-database-url and itsdangerous are single module packages - pyyaml and httplib2 use 2 separate directories for Py2 vs Py3 sources - matplotlib has an src/ which contains only C code, a lib/ directory for Python code and a long setup.py

If you look at the table above, most projects don't use the src/ layout. I'm not sure if that matters all that much but then I spend a fair amount of time on this so, I'm just going to leave this list here. :P