Skip to content

Add support for pyproject.toml files when adding --editable projects …#7670

Open
ughstudios wants to merge 8 commits intopython-poetry:mainfrom
ughstudios:master
Open

Add support for pyproject.toml files when adding --editable projects …#7670
ughstudios wants to merge 8 commits intopython-poetry:mainfrom
ughstudios:master

Conversation

@ughstudios
Copy link

@ughstudios ughstudios commented Mar 17, 2023

There is currently a problem where setup_reader.py can only read setup.cfg or setup.py files for adding local packages. This adds support for flitcore and other backends that are defined in pyproject.toml files

Resolves: #7669

Add support for pyproject.toml files when adding --editable projects …
5f229f0 
…that are on local disk.

There is currently a problem where setup_reader.py can only read setup.cfg or setup.py files for adding local packages. This adds support for flitcore and other backends that are defined in pyproject.toml files
@dimbleby
Copy link
Contributor

dimbleby commented Mar 17, 2023 •
edited
Loading

immediate thoughts

  • general idea seems good, pep-621 compliant pyproject.toml are beginning to appear in the wild and it's good to get a first step in supporting them
  • this code certainly doesn't want to be in a class called SetupReader, it's not dealing with setup files, suggest instead introducing something new and refactoring PackageInfo.from_directory()
  • I don't think this should be tied to flit: pep 621 is a standard and other backends support it
  • I'd anti-recommend using tomlkit to parse pyproject.toml here. poetry already has tomllib in its dependencies - that'll be faster and also sidestep the type-checking issues currently visible in the pipeline
  • needs testcases

@ughstudios
Copy link
Author

ughstudios commented Mar 17, 2023

I'd anti-recommend using tomlkit to parse pyproject.toml here. poetry already has tomllib in its dependencies - that'll be faster and also sidestep the type-checking issues currently visible in the pipeline

The codebase I work on is only 3.10, do you mind if I add a fallback to use tomlkit for versions <3.11? tomllib is only available in 3.11.

@dimbleby
Copy link
Contributor

dimbleby commented Mar 17, 2023

The codebase I work on is only 3.10, do you mind if I add a fallback to use tomlkit for versions <3.11? tomllib is only available in 3.11.

from poetry.utils._compat import tomllib

@ughstudios
Copy link
Author

ughstudios commented Mar 17, 2023 •
edited
Loading

Is there a way to run these tests at my desk so I don't keep spamming you? NVM I guess I can just run each of them manually.

@ughstudios
Copy link
Author

ughstudios commented Mar 17, 2023

I'm running through all of the tests in the test suite. Are any of the tests known to be broken? It seems like my change has broken like 10+ tests :(

@dimbleby
Copy link
Contributor

dimbleby commented Mar 17, 2023

If you've figured out how to run the tests then you can very easily verify for yourself that they pass without your changes.

On the plus side this probably points you in the right direction for figuring out where to add new tests.

@dimbleby dimbleby mentioned this pull request Mar 22, 2023
Closed
2 tasks
FFY00
FFY00 approved these changes Mar 23, 2023
View reviewed changes
Copy link

@FFY00 FFY00 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. I think the test failures are just due to the test suite relying too much on mocks, making it fragile when we touch the internals. I am also getting them in #7703, but haven't gotten around to look at them.

src/poetry/inspection/info.py
else:
info = get_pep517_metadata(path)
except PackageInfoError:
info = cls.from_pyproject_toml(path)
Copy link

@FFY00 FFY00 Mar 23, 2023

Choose a reason for hiding this comment

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

IMO, instead of putting this here, we should probably create a from_project_static_files which tries from_pyproject_toml and then from_setup_files, and then replace all existing from_setup_files usages with from_project_static_files.

Copy link
Author

@ughstudios ughstudios Mar 25, 2023

Choose a reason for hiding this comment

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

I'll have to do this on Monday when I'm back at my desk. Are you one of the maintainers of poetry? It says "FFY00 approved these changes 3 days ago."

@dimbleby dimbleby mentioned this pull request Mar 2, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@FFY00 FFY00 FFY00 approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support reading metadata from PEP 621 compliant pyproject.toml

3 participants

@ughstudios @dimbleby @FFY00