Removing tomlkit from poetry core

[dimbleby]

Extended version of #6562, companion to python-poetry/poetry-core#483

Updates poetry to work with poetry-core after tomlkit is removed from there.

[radoering]

Eventually, making this PR backward compatible was easier than I thought first. This gives us a clear procedure for merging this PR and its companion and allows us to further run downstream tests against poetry's master in poetry-core after merging.

IMO (and I don't recall contrary opinions), there is no need for tomlkit in poetry-core since we do not (and probably will never) write toml files in core. Since tomli is included in Python 3.11 as tomllib, it makes sense to rely on it if it fulfills our needs, especially in poetry-core.

I'll try to do a thorough review at the end of the week (or next week) and if there are no objections by then move forward.

[dimbleby]

making this PR backward compatible

nice. On the whole I'd prefer the world to be arranged so that such things weren't necessary (approximately: disentangle the pipelines, rely on version numbers to signal compatibility) but it's certainly helpful that this can be done now

IMO ... there is no need for tomlkit in poetry-core

agreed: apart from the pipeline awkwardness - apparently now addressed - I think this ought to be uncontroversial

thanks for reviving this one

[radoering]

Poetry.file is probably used by some plugins, so I'd prefer not to change its type. Of course, its core equivalent can't return a TOMLFile anymore. However, we could rename it e.g. to pyproject_path in core.

[dimbleby]

Poetry.file is probably used by some plugins, so I'd prefer not to change its type. Of course, its core equivalent can't return a TOMLFile anymore. However, we could rename it e.g. to pyproject_path in core.

If we remove (rename) poetry.file then that's breaking for all the places that access poetry.file.

The evidence from poetry itself is that there are lots of places that get poetry.file and use it only as a Path (mostly wanting poetry.file.parent).

So I think this is not an improvement.

[radoering]

If we remove (rename) poetry.file then that's breaking for all the places that access poetry.file.

That applies only to downstream projects that depend on poetry-core directly but not on poetry. I'm not sure that's even a use case. I'd expect that plugin developers will rely on poetry.poetry.Poetry (which should keep poetry.file) not on poetry.core.poetry.Poetry. IMO we might even do a major version bump of poetry-core, but I'd like to avoid a breaking change for plugin developers.

[dimbleby]

I'm not following, sorry!

Is the suggestion that

• in poetry.core, rename poetry.file -> poetry.pyproject_path
• in poetry, rename poetry.toml_file -> poetry.file
  ?

that will give up backwards compatibility in this MR (because poetry.core.file is no longer available).

or did you mean something else?

[radoering]

I think we were talking past each other? Sorry for being ambiguous. I hope the two in-code-comments (the first in the core PR) make my suggestion more clear?

[dimbleby]

I see the suggestion, but my point is that the poetry-core change will be breaking for poetry itself. ie we'll back either with pipelines temporarily broken while one but not the other is merged, or doing some more complicated dance of renames and releases to avoid that.

Did I misunderstand? is that your intention?

[radoering]

I can't follow where it will be breaking for poetry itself. The only place in poetry where poetry.core.poetry.Poetry is used is to inherit from it. If we add the file property to poetry's Poetry class we can keep using it as before. It doesn't matter if poetry-core's Poetry class has the same file property (current state) or no file property at all (future), does it? (For backward compatibility we should use the file property and not the new pyproject_path property, which is only in the future poetry-core, but that's something I'm willing to do for now.)

[dimbleby]

it's breaking in the following places:

src/poetry/factory.py:59: error: "Poetry" has no attribute "file"  [attr-defined]
src/poetry/factory.py:66: error: "Poetry" has no attribute "file"  [attr-defined]
src/poetry/factory.py:85: error: "Poetry" has no attribute "file"  [attr-defined]
src/poetry/masonry/builders/editable.py:109: error: "Poetry" has no attribute "file"  [attr-defined]
src/poetry/masonry/builders/editable.py:115: error: "Poetry" has no attribute "file"  [attr-defined]
src/poetry/masonry/builders/editable.py:136: error: "Poetry" has no attribute "file"  [attr-defined]
src/poetry/masonry/builders/editable.py:148: error: "Poetry" has no attribute "file"  [attr-defined]
src/poetry/masonry/builders/editable.py:155: error: "Poetry" has no attribute "file"  [attr-defined]
src/poetry/masonry/builders/editable.py:169: error: "Poetry" has no attribute "file"  [attr-defined]
src/poetry/masonry/builders/editable.py:251: error: "Poetry" has no attribute "file"  [attr-defined]
src/poetry/installation/pip_installer.py:229: error: "Path" has no attribute "path"  [attr-defined]
src/poetry/installation/executor.py:658: error: "Path" has no attribute "path"  [attr-defined]

[dimbleby]

I suppose these can be worked around with a some casts and hasattr and suchlike

[dimbleby]

Is it better? I dunno, it's different anyway

[radoering]

On it's own it's just different but I hope it makes the transition transparent for plugin developers.

Maybe, we should do the same for PyProject for consistency?

• in poetry: toml_file -> file
• in core: file -> path

[dimbleby]

Maybe, we should do the same for PyProject

sure

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.