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

Support namespace packages with new command and misc. improvements #2768

Merged
merged 7 commits into from Apr 12, 2021

Conversation

abn
Copy link
Member

@abn abn commented Aug 4, 2020

Improve supported layout creation

  • remove unnecessary template duplication
  • stop generating default test case (this is out of scope for poetry)
  • stop generating content for package __init__.py
  • simplify inheritance to only require base directory overrides
  • remove "StandardLayout" (replaced with Layout)
  • support handling namespace package names given in the form "a.b.c"
  • specify include packages by default (be explicit)

Support namespace package creation (via --name)

With this change, names containing "." are treated as namespace
packages. The following behaviour is now expected.

  • "a.b.c" creates package name "a-b-c" with directory structure "a/b/c"
  • "a-b-c" creates package name "a-b-c" with directory structure "a_b_c"
  • "a.b_c" creates package name "a-b-c" with directory structure "a/b_c"
  • "a_b_c" creates package name "a-b-c" with directory structure "a_b_c"

Add --readme option with default fromat markdown

Previously, the new command created a .rst readme files as default. This is now changed to use markdown. Additionally,
a new option --readme has been added. (changes ported from #1515 by @finswimmer).

Closes: ##1515 #280

Misc. changes

  • do not add pytest dev dependency as this should be left upto the end-user and can be achieved using the cli
  • fix handling of input path when it is not relative
  • add test coverage for command

@abn abn requested a review from a team August 4, 2020 17:16
Copy link
Member

@finswimmer finswimmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall a good work 👍 I like the idea of creating namespace packages.

I added some questions and opinions.

poetry/console/commands/new.py Show resolved Hide resolved
poetry/console/commands/new.py Outdated Show resolved Hide resolved
poetry/layouts/layout.py Outdated Show resolved Hide resolved
poetry/layouts/layout.py Outdated Show resolved Hide resolved
poetry/layouts/layout.py Show resolved Hide resolved
Copy link
Member

@sdispater sdispater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"a-b-c" creates package name "a-b-c" with directory structure "a/b/c"

I am not sure we want this, and it's a change to the existing behavior. Putting a dash in the package name is common but does not necessarily mean we want a namespace package.

@abn
Copy link
Member Author

abn commented Aug 5, 2020

I am not sure we want this, and it's a change to the existing behavior. Putting a dash in the package name is common but does not necessarily mean we want a namespace package.

Personally, I reckon this is a better default. Especially since we can do poetry new --name a_b_c a-b-c we can also add a confirmation step if required.

@abn
Copy link
Member Author

abn commented Aug 6, 2020

@sdispater I have left the current beahviour as it is for now; but supporting --name a.b.c as namespace packages.
@finswimmer all your recoomendations have been incorporated (except the min vesrsion one).

@abn
Copy link
Member Author

abn commented Aug 27, 2020

@python-poetry/core this is rebased and ready for review again

Copy link
Member

@finswimmer finswimmer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some minor points about type hinting :)

poetry/layouts/layout.py Outdated Show resolved Hide resolved
poetry/layouts/layout.py Outdated Show resolved Hide resolved
@abn
Copy link
Member Author

abn commented Sep 1, 2020

Good ol' type hinting. Fixed :)

@abn abn requested a review from finswimmer September 1, 2020 22:42
@abn abn linked an issue Sep 2, 2020 that may be closed by this pull request
3 tasks
@abn
Copy link
Member Author

abn commented Sep 16, 2020

@finswimmer can you have a look at this one again please?

@abn abn added this to In progress in 1.1 via automation Sep 23, 2020
1.1 automation moved this from In progress to Review in progress Sep 23, 2020
1.1 automation moved this from Review in progress to Reviewer approved Sep 25, 2020
sdispater
sdispater previously approved these changes Sep 25, 2020
Copy link
Member

@sdispater sdispater left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍

However, it will have to wait the 1.1 release before being merged (I put it into the 1.2 project).

@sdispater sdispater added this to In progress in 1.2 via automation Sep 25, 2020
@sdispater sdispater removed this from Reviewer approved in 1.1 Sep 25, 2020
@sdispater sdispater added this to the 1.2 milestone Sep 25, 2020
@abn abn moved this from In progress to Reviewer approved in 1.2 Sep 25, 2020
@abn
Copy link
Member Author

abn commented Sep 25, 2020

Sigh :( I hoped to get this in this release. 1.2 it is!

1.2 automation moved this from Reviewer approved to Review in progress Apr 9, 2021
@abn abn requested a review from sdispater April 9, 2021 22:13
@abn
Copy link
Member Author

abn commented Apr 9, 2021

@sdispater this is ready again; CI might have failures that should be fixed once #3900 is merged.

@abn abn requested review from a team and removed request for sdispater and finswimmer April 9, 2021 22:14
abn and others added 7 commits April 11, 2021 01:02
- remove unnecessary template duplication
- stop generating default test case (this is out of scope for poetry)
- stop generating content for package `__init__.py`
- simplify inheritance to only require base directory overrides
- remove "StandardLayout" (replaced with `Layout`)
- support handling namespace package names given in the form "a.b.c"
- specify include packages by default (be explicit)
With this change, names containing "." are treated as namespace
packages.

The following behaviour is now expected.

- "a.b.c" creates package name "a-b-c" with directory structure
  "a/b/c"
- "a-b-c" creates package name "a-b-c" with directory structure
  "a/b/c"
- "a.b_c" creates package name "a-b-c" with directory structure
  "a/b_c"
- "a_b_c" creates package name "a-b-c" with directory structure
  "a_b_c"
Relates-to: python-poetry#280

Co-authored-by: Arun Babu Neelicattu <arun.neelicattu@gmail.com>
This change makes readme formant configurable, defaulting to markdown
when using the new command.

Resolves: python-poetry#280
Closes: python-poetry#1515

Co-authored-by: Arun Babu Neelicattu <arun.neelicattu@gmail.com>
1.2 automation moved this from Review in progress to Reviewer approved Apr 12, 2021
@abn abn merged commit affabe0 into python-poetry:master Apr 12, 2021
1.2 automation moved this from Reviewer approved to Done Apr 12, 2021
@abn abn deleted the improve-command-new branch April 12, 2021 13:29
@sdispater sdispater mentioned this pull request Apr 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
1.2
  
Done
Development

Successfully merging this pull request may close these issues.

The Pytest module automatically installed by Poetry is outdated
4 participants