Support import-names according to PEP-794 #894
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR adds PEP-794 import-names and import-namespaces support by extending package and metadata models, bumping the metadata version to 2.5, wiring those fields through the build process, validating them in the factory, and updating tests and fixtures accordingly. Sequence diagram for validation of import_names and import_namespaces in FactorysequenceDiagram
participant Factory
participant ProjectPackage
participant Error
Factory->>ProjectPackage: get("import-names")
Factory->>ProjectPackage: get("import-namespaces")
Factory->>Factory: _validate_import_names(project)
alt Duplicate names found
Factory->>Error: raise PoetryCoreError
end
Entity relationship diagram for import_names and import_namespaces fields in package metadataerDiagram
PROJECT_PACKAGE {
string name
string version
list import_names
list import_namespaces
}
METADATA {
string metadata_version
list import_names
list import_namespaces
}
PROJECT_PACKAGE ||--|| METADATA: "generates"
Class diagram for updated ProjectPackage and Metadata classes (PEP-794 support)classDiagram
class ProjectPackage {
+import_names: list[str] | None
+import_namespaces: list[str] | None
}
class Metadata {
+metadata_version: str
+import_names: list[str] | None
+import_namespaces: list[str] | None
+from_package(package: ProjectPackage): Metadata
}
ProjectPackage <--> Metadata: used by
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
finswimmer
force-pushed
the
pep794-import-names
branch
2 times, most recently
from
eb61027 to
6497c57
Compare
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- You didn’t update the JSON project schema (project-schema.json) to allow import-names and import-namespaces under the [project] table, so schema validation will fail when parsing those keys.
- In Builder.get_metadata_content you special-case empty import lists to emit blank headers—consider removing that branch and only emitting Import-Name/Import-Namespace for non-empty entries.
- Factory._validate_import_names strips off any qualifiers (the part after “;”) before checking duplicates, which can produce misleading error messages; consider validating against the full strings or adjusting the error text.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You didn’t update the JSON project schema (project-schema.json) to allow import-names and import-namespaces under the [project] table, so schema validation will fail when parsing those keys.
- In Builder.get_metadata_content you special-case empty import lists to emit blank headers—consider removing that branch and only emitting Import-Name/Import-Namespace for non-empty entries.
- Factory._validate_import_names strips off any qualifiers (the part after “;”) before checking duplicates, which can produce misleading error messages; consider validating against the full strings or adjusting the error text.
## Individual Comments
### Comment 1
<location> `src/poetry/core/masonry/builders/builder.py:293-298` </location>
<code_context>
content += f"\n{self._meta.description}\n"
+ if self._meta.import_names is not None:
+ if len(self._meta.import_names) == 0:
+ content += "Import-Name: \n"
+
+ for import_name in self._meta.import_names:
</code_context>
<issue_to_address>
**suggestion:** Writing an empty Import-Name field may be unnecessary.
Omit 'Import-Name:' when the list is empty to keep the metadata cleaner.
```suggestion
if self._meta.import_names is not None:
for import_name in self._meta.import_names:
content += f"Import-Name: {import_name}\n"
```
</issue_to_address>
### Comment 2
<location> `src/poetry/core/masonry/builders/builder.py:300-305` </location>
<code_context>
+ content += f"Import-Name: {import_name}\n"
+
+ if self._meta.import_namespaces is not None:
+ if len(self._meta.import_namespaces) == 0:
+ content += "Import-Namespace: \n"
+
+ for namespace in self._meta.import_namespaces:
</code_context>
<issue_to_address>
**suggestion:** Empty Import-Namespace field may be omitted for clarity.
Consider omitting the Import-Namespace field entirely when the list is empty, as is done for Import-Name, to improve metadata clarity.
```suggestion
if self._meta.import_namespaces is not None:
for namespace in self._meta.import_namespaces:
content += f"Import-Namespace: {namespace}\n"
```
</issue_to_address>
### Comment 3
<location> `tests/masonry/test_metadata.py:208-217` </location>
<code_context>
assert metadata.description == "Description 1\nDescription 2"
[email protected](
+ ["import_names", "import_namespaces"],
+ [
+ (["my_package.a", "my_package.b"], ["my_package"]),
+ (None, None),
+ ([], []),
+ ],
+)
+def test_from_package_import_names(
+ import_names: list[str] | None,
+ import_namespaces: list[str] | None,
+ package: ProjectPackage,
+) -> None:
+ package.import_names = import_names
+ package.import_namespaces = import_namespaces
+
+ metadata = Metadata.from_package(package)
+
+ assert metadata.import_names == import_names
+ assert metadata.import_namespaces == import_namespaces
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test for invalid or malformed import-names/import-namespaces values.
Adding tests for malformed or unexpected values, such as non-string items, whitespace-only strings, or duplicates, will help verify that the Metadata model correctly handles or rejects these cases.
Suggested implementation:
```python
@pytest.mark.parametrize(
["import_names", "import_namespaces"],
[
(["my_package.a", "my_package.b"], ["my_package"]),
(None, None),
([], []),
],
)
def test_from_package_import_names(
import_names: list[str] | None,
import_namespaces: list[str] | None,
package: ProjectPackage,
) -> None:
package.import_names = import_names
package.import_namespaces = import_namespaces
metadata = Metadata.from_package(package)
assert metadata.import_names == import_names
assert metadata.import_namespaces == import_namespaces
@pytest.mark.parametrize(
["import_names", "import_namespaces", "expected_exception"],
[
([123, "my_package.b"], ["my_package"], TypeError), # Non-string in import_names
([" ", "my_package.b"], ["my_package"], None), # Whitespace-only string
(["my_package.a", "my_package.a"], ["my_package"], None), # Duplicate entries
(["my_package.a"], [None], TypeError), # Non-list in import_namespaces
(["my_package.a"], [" "], None), # Whitespace-only string in namespaces
],
)
def test_from_package_import_names_malformed(
import_names,
import_namespaces,
expected_exception,
package: ProjectPackage,
) -> None:
package.import_names = import_names
package.import_namespaces = import_namespaces
if expected_exception:
with pytest.raises(expected_exception):
Metadata.from_package(package)
else:
metadata = Metadata.from_package(package)
# Check for duplicates and whitespace-only strings
if import_names is not None:
assert all(isinstance(name, str) for name in metadata.import_names)
assert all(name.strip() != "" for name in metadata.import_names)
assert len(set(metadata.import_names)) == len(metadata.import_names)
if import_namespaces is not None:
assert all(isinstance(ns, str) for ns in metadata.import_namespaces)
assert all(ns.strip() != "" for ns in metadata.import_namespaces)
assert len(set(metadata.import_namespaces)) == len(metadata.import_namespaces)
```
If the `Metadata.from_package` method does not currently raise exceptions or handle these cases, you will need to update its implementation to validate input types, remove whitespace-only strings, and deduplicate entries. Adjust the assertions in the test as needed to match the actual behavior.
</issue_to_address>
### Comment 4
<location> `tests/masonry/builders/test_builder.py:180-184` </location>
<code_context>
"Repository, https://github.com/python-poetry/poetry",
]
+ import_names = parsed.get_all("Import-Name", False)
+ assert import_names is False
+
+ import_namespaces = parsed.get_all("Import-Namespace", False)
+ assert import_namespaces is False
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test case where import-names and import-namespaces are present in the metadata.
Adding such a test will ensure correct handling and parsing when these fields are present.
```suggestion
import_names = parsed.get_all("Import-Name", False)
assert import_names is False
import_namespaces = parsed.get_all("Import-Namespace", False)
assert import_namespaces is False
# Test case: metadata with Import-Name and Import-Namespace present
metadata_with_imports = (
"Metadata-Version: 2.1\n"
"Name: my-package\n"
"Version: 1.2.3\n"
"Summary: Some description.\n"
"Import-Name: my_package\n"
"Import-Name: another_package\n"
"Import-Namespace: my_namespace\n"
"Import-Namespace: another_namespace\n"
)
parsed_with_imports = p.parsestr(metadata_with_imports)
import_names_present = parsed_with_imports.get_all("Import-Name")
import_namespaces_present = parsed_with_imports.get_all("Import-Namespace")
assert import_names_present == ["my_package", "another_package"]
assert import_namespaces_present == ["my_namespace", "another_namespace"]
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
radoering
requested changes
Nov 2, 2025
Member
radoering
left a comment
radoering
left a comment
There was a problem hiding this comment.
Apart from some pedantic feedback in the separate comments I wonder if we should determine the core metadata fields from other information we have if the import names are not defined in the project section or if we do not want to go down this rabbit hole (or at least defer it)?
finswimmer
force-pushed
the
pep794-import-names
branch
from
b3ffd2b to
2bfff16
Compare
radoering
reviewed
Nov 15, 2025
finswimmer
force-pushed
the
pep794-import-names
branch
from
2bfff16 to
d4c5aeb
Compare
radoering
reviewed
Nov 17, 2025
finswimmer
force-pushed
the
pep794-import-names
branch
from
2ff10a0 to
beced2e
Compare
Member
|
I wonder whether we should wait for a packaging release with pypa/packaging#948. There is already an issue (pypa/packaging#950). At the moment, packaging can parse raw metadata ( By the way, pkginfo does not support 2.5 either, but handles it more gracefully: |
Member
|
Blocker: pypi/warehouse#19083 Attention: I was able to upload a package with metadata version 2.5 to TestPyPI by using the latest Poetry release for publishing because it claims the metadata version is 2.4 (because that would be the version if you built with the included poetry-core). A new Poetry release (or a tool that uses the actual metadata version of a build artifact) would not be able to upload a package. |
Merged
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Implement PEP-794 support for import-names and import-namespaces by extending package metadata models, updating the builder to emit the new fields, bumping the metadata version to 2.5, and adding corresponding tests.
New Features:
Enhancements:
Tests:
Summary by Sourcery
Implement PEP-794 import-names support by extending package metadata, builder, and factory logic, bump metadata version to 2.5, and add tests for import-names and import-namespaces.
New Features:
Enhancements:
Tests: