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

--script and --lib flags #738

Merged
merged 5 commits into from Feb 23, 2024
Merged

--script and --lib flags #738

merged 5 commits into from Feb 23, 2024

Conversation

dsp
Copy link
Contributor

@dsp dsp commented Feb 21, 2024

  • fix missing space in init --build-system flag description

  • init: add support for --bin
    --lib and --lib-rs,This adds support for three new flags which determine the templates
    we generate:

  1. We adding --lib, which will generate the standard python library
    We no longer generate [project.scripts] section and updated
    the template.
  2. We added --script, which will generate a package with a main.py,
    init.py and an entry to [project.scripts]. __main__.py follows
    semantics found in Python packages such as ensurepip and `pip.
  3. We added simple tests. Tests do not cover yet --lib and --build-system=maturin.
  4. We added a short section about executable projects to docs/guide/basic.md
  5. We moved all in init.rs to a templates folder and use include_str!().

@dsp dsp changed the title dsp/bin lib flags --bin, --lib, --lib-rs flags Feb 21, 2024
@dsp
Copy link
Contributor Author

dsp commented Feb 21, 2024

Adding this as a draft to get feedback, while I am working to add tests.

@mitsuhiko
Copy link
Collaborator

I think this looks good, I'm however not sure if --lib-rs is a good idea. I do however also recognize that this build system thing is odd / verbose.

@dsp
Copy link
Contributor Author

dsp commented Feb 22, 2024

I think this looks good, I'm however not sure if --lib-rs is a good idea. I do however also recognize that this build system thing is odd / verbose.

My reasoning here is that these options determine the template used. The intuition here is that I likely know the kind of project i want, but leave the specifics of build system choice, etc to rye to choose. This means that rye has "sane default' switch for a template. For instance --lib and --bin implying hatchling and --lib-rs implying maturin. If in the future some new build system comes out and rye chooses this, we can flip the switch to have --lib-rs imply something else.

@dsp
Copy link
Contributor Author

dsp commented Feb 22, 2024

The counter argument btw is that we want to implment a PyO3/Maturin template for --bin as well. Instead of then having an awkward --bin-rs we just have any combination of --build-system and --bin and --lib flags.

So our choices are effectively:

  1. Only keep --bin and --lib, build system choises the specific template choosen.
  2. Add --lib-rs and treat them all as explicit template choices. This means later we might need --bin-rs, etc.
  3. Some alternative solution (e.g. --lib=rust)
    Happy to go with whatever your preferred choice for rye is. The more i think, the more I actually think adding a template for the --bin --build-system=maturin case is the best choice and just go with --lib and --bin.

Notable I am not happy with --bin because technically it's not a binary that we are producing and thought about --script or --executable. I am not sure about a good alternative.

@mitsuhiko
Copy link
Collaborator

I would go with --lib and --bin and then use the build system for the template. It can always error out and say it's not a sensible combination. I also agree that --bin is weird. Maybe it should be called --script instead?

@dsp dsp force-pushed the dsp/bin-lib-flags branch 4 times, most recently from 5ad1eaf to 02290ec Compare February 22, 2024 16:14
@dsp dsp marked this pull request as ready for review February 22, 2024 16:17
This adds support for three new flags which determine the templates
we generate:

  1. We adding --lib, which will generate the standard python library
     We no longer generate `[project.scripts]` section and updated
     the template.
  2. We added --script, which will generate a package with a __main__.py,
     __init__.py and an entry to `[project.scripts]`. `__main__.py` follows
     semantics found in Python packages such as `ensurepip` and `pip.
@dsp
Copy link
Contributor Author

dsp commented Feb 22, 2024

  • added tests.
  • added changelog entry.
  • added an entry to docs/guide/basic.md
  • refactored how we deal with templates in init.rs (this is deliberately the last commit and can be dropped, if needed)

@dsp dsp changed the title --bin, --lib, --lib-rs flags --script and --lib flags Feb 22, 2024
@@ -0,0 +1,4 @@
import {{ name_safe }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know people love __main__ but do we think it's a good idea? At least personally I really try to avoid it. Not sure how others feel about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked into common Python packages like ensurepip, pip, etc. They all provide __main__.py so that you can do python -m pip for example. I think it is good to provide a way so people can run python -m my-package from a rye project. That's why i included it as a thin wrapper around init

rye/src/cli/init.rs Outdated Show resolved Hide resolved
rye/src/templates/LICENSE.txt Outdated Show resolved Hide resolved
We make init.rs a bit more readable by moving the templates to a new
folder templates/ which we organise by template type (lib, script) and
build system.
@dsp dsp force-pushed the dsp/bin-lib-flags branch 2 times, most recently from 10c3463 to 51887db Compare February 22, 2024 21:00
@mitsuhiko mitsuhiko merged commit 4cbb1d4 into astral-sh:main Feb 23, 2024
6 checks passed
@mitsuhiko
Copy link
Collaborator

This looks good!

@dsp dsp deleted the dsp/bin-lib-flags branch February 23, 2024 09:47
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.

None yet

2 participants