Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: allow src dir #260

Merged
merged 7 commits into from
Nov 3, 2019
Merged

WIP: allow src dir #260

merged 7 commits into from
Nov 3, 2019

Conversation

okken
Copy link
Contributor

@okken okken commented Apr 24, 2019

I'd really like to use flit for projects with src directories.

The flit project already has 2 types of projects.

  1. just a module, like foo.py
  2. a package (directory with __init__.py), like foo/__init__.py

This would add a 3rd and 4th.

  1. just a module, but in src, like src/foo.py
  2. a package in src, like src/foo/__init__.py

It has minimal impact on the rest of the code, only touching 2 functions.
All existing tests still pass.

@okken
Copy link
Contributor Author

okken commented Apr 24, 2019

Added some tests.

@okken
Copy link
Contributor Author

okken commented Apr 24, 2019

Ok. I see that some code coverage targets aren't getting hit.
I can work on those, if required for the PR to move forward. Just let me know.

@okken okken changed the title allow src dir WIP: allow src dir Apr 24, 2019
@theacodes
Copy link
Member

My god, we could settle pypa/packaging.python.org#320 finally.

@astrojuanlu
Copy link

If I understood correctly, @takluyver concern was not the implementation itself, but the possibility of confusing the users by offering too many options before the discussion is settled.

(For further info, pypa/packaging.python.org#320 and especially pypa/packaging.python.org#320)

@pradyunsg
Copy link
Member

@theacodes, I don't think this changes anything since @takluyver stated that he doesn't want users to have additional knobs when using flit.

#115 (comment)

@okken
Copy link
Contributor Author

okken commented Apr 28, 2019

No extra nobs. It works without changing existing functionality and without adding any new options.

@okken
Copy link
Contributor Author

okken commented Apr 28, 2019

Regarding confusing users. It is inevitable, given the benefits of using src, that some tool(s) will develop that support src. It will be less confusing to users to have flit support both models than to have to choose between two different tools.

@glyph
Copy link

glyph commented Aug 3, 2019

@okken Any luck on getting the test coverage up to snuff?

@jugmac00
Copy link

@okken Haven't you mentioned somewhere that flit already works with src directories? Could you link it here?

@okken
Copy link
Contributor Author

okken commented Aug 17, 2019

Flit does not work with src directories. However, I don’t think it needs to. It would be cool if it had this option. But I’ve convinced myself that the major reasons to use src are solvable by running tests from the tests directory. I’ve not written it up, but I discussed it here: https://testandcode.com/80

@geryogam
Copy link

geryogam commented Sep 9, 2019

A lot of major Python projects have switched to the src layout now:

And a few major Python projects have adopted a non-conventional lib layout:

So while this PR improve the situation, it would be better if we add an optional -s/--source-dir command-line parameter to give users the choice where they put their source code in my opinion. It is the single most important option for the user: "I want to package THIS code", so it should be available. Flit support options that are not essential, such as -f/--ini-file or --python, so I don't think the "too many command-line parameters" argument holds.

Opinionated is fine but not in this case.

@okken
Copy link
Contributor Author

okken commented Sep 9, 2019

@glyph, re "> @okken Any luck on getting the test coverage up to snuff?"

I did this PR as a proof of concept.
I was actually very surprised that something as central as flit had such low coverage as it does.
I'm not interested in increasing test coverage for this PR if @takluyver is going to reject it anyway.

@takluyver If this PR was updated to increase the code coverage, would consider merging it?

@okken
Copy link
Contributor Author

okken commented Sep 9, 2019

I've previously stated "Flit does not work with src directories. However, I don’t think it needs to."
I've changed my mind on this.
There are reasons other than tox testing to use src structure, and it should be allowed.

@glyph
Copy link

glyph commented Nov 2, 2019

@takluyver (any word on @okken's question re: coverage?)

@okken
Copy link
Contributor Author

okken commented Nov 3, 2019

I've extended the testing to cover all the changes I made.
I'd love to see the changes merged.

@okken
Copy link
Contributor Author

okken commented Nov 3, 2019

@okken Any luck on getting the test coverage up to snuff?

yes. coverage is back up to the target

@okken
Copy link
Contributor Author

okken commented Nov 3, 2019

The test reason for using src has workarounds other than using src.
However, structurally, it's still often nice to have a src directory.
As many projects have either started or switched to src structure, I'm obviously not the only one that thinks it's a good idea.

@pradyunsg
Copy link
Member

I realize I might've sounded skeptical in my previous comment (#260 (comment)), so clarifying -- I do think this is an improvement that would be good to have in flit.

This is essentially a superset of the functionality that I'd want in flit so I'm very much on board for this.

@takluyver takluyver merged commit ea347ed into pypa:master Nov 3, 2019
@takluyver
Copy link
Member

Sorry I've been silent for so long, and thanks for being patient. I'm going to try to get to Flit 2.0 in the next week or so.

@takluyver
Copy link
Member

Flit 2.0rc2 is now out (rc1 went a bit wrong). See the release notes, and please give it a test drive by installing with pip install --pre flit.

Until a final release is out, you'll need to manually adjust pyproject.toml to allow pre-release versions of flit_core for the build backend:

[build-system]
requires = ["flit_core >=2.0rc2,<3"]
build-backend = "flit_core.buildapi"

@ju-sh
Copy link

ju-sh commented Aug 21, 2020

Hi. How can I create a new package with src layout with flit?

I couldn't find the way in the docs.

I'm using flit 2.3.0.

Edit: I think I get it now. We can just create the layout by ourselves and flit will find the files accordingly. Is that it?

@okken
Copy link
Contributor Author

okken commented Aug 21, 2020 via email

@ju-sh
Copy link

ju-sh commented Aug 21, 2020

Is there a part in docs which mentions package creation with src layout? If not, can it be added? I could find brief descriptions of it under release history but not much.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants