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

Improve poetry file #384

Merged
merged 2 commits into from
Mar 7, 2024
Merged

Improve poetry file #384

merged 2 commits into from
Mar 7, 2024

Conversation

ErikDeSmedt
Copy link
Collaborator

A general clean-up of the pyproject.toml-files.
It is now possible to install all dependencies using poetry install.

Before you had to do

cd docs; poetry install
cd libs/gl-client-py; poetry install
cd libs/gl-testing; poetry install

Another advantage is that it now properly resolves dependencies and
conflicts. In the old approach conflicts between two different
pyproject.toml-files wouldn't be detected.

Users who want to contribute to the documentation have to use

poetry install --with docs

@ErikDeSmedt ErikDeSmedt marked this pull request as ready for review March 7, 2024 13:05
A general clean-up of the `pyproject.toml`-files.
It is now possible to install all dependencies using `poetry install`.

Before you had to do
```
cd docs; poetry install
cd libs/gl-client-py; poetry install
cd libs/gl-testing; poetry install
```

Another advantage is that it now properly resolves dependencies and
conflicts. In the old approach conflicts between two different
`pyproject.toml`-files wouldn't be detected.

Users who want to contribute to the documentation have to use

```
poetry install --with docs
```
@nepet
Copy link
Collaborator

nepet commented Mar 7, 2024

In case anybody is getting an error similar to 'name' when running poetry shell -> updating poetry to 1.8.2 helped: poetry self update

Copy link
Collaborator

@nepet nepet left a comment

Choose a reason for hiding this comment

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

ack bddb115

@cdecker
Copy link
Collaborator

cdecker commented Mar 7, 2024

Good proposal, we should definitely have a top-level pyproject.toml file, that then pulls in the direct dependencies only, and the sub-projects that the main project requires. I don't like pulling in the docs dependencies into the main dependencies too much.

We could instead of moving the doc-dependencies into the main pyproject.toml file, rather add a dependency on docs (and rename it to something that doesn't clash with PyPI such as gl-internal-docs`) and then that one should have all its dependencies.

@ErikDeSmedt
Copy link
Collaborator Author

ErikDeSmedt commented Mar 7, 2024

I don't like pulling in the docs dependencies into the main dependencies too much.

Initially, I followed your sentiment but I realized the maintenance burden is just too high.

I see two reasons why you would want to create a separate pyproject.toml-file

  1. You want to do cd docs; poetry install and have a working environment
  2. You want to package the project

We want to package and publish gl-client and gl-testing thus we need a pyproject.toml-file.
We don't want to package the docs so we don't need a pyproject.toml-file.

How it increases maintenance burden?

In poetry dev-dependencies are local to a single project and poetry never installs dev-dependencies from a dependency.

If we want to add a type-stub as a dev-dependency to gl-client-py we would have to add it to

  • ./libs/gl-client-py/pyproject.toml
    Because cd ./libs/gl-client-py; poetry install should install the dependency
  • ./pyproject.toml
    Because poetry install should install the dependency
  • ./docs/pyproject.toml
    Because cd docs; poetry install should install the dependency. We should build the reference using typestubs

I don't want to create a pyproject.toml-file that we'll never use and that get's outdated easily.

Groups are recommended

The first paragraph in the docs about dependency management is

Poetry provides a way to organize your dependencies by groups. For instance, you might have dependencies that are only needed to test your project or to build the documentation.

@cdecker
Copy link
Collaborator

cdecker commented Mar 7, 2024

Excellent rationale, thank you. I think I had not fully understood the intent and now that I do I fully agree 👍

ACK bddb115

@cdecker cdecker merged commit e91c156 into main Mar 7, 2024
18 checks passed
@cdecker cdecker deleted the poetry-file branch March 7, 2024 22:28
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.

3 participants