Skip to content

Fix Poetry lock optional dependency parsing - #5295

Open
Mars-60 wants to merge 2 commits into
aboutcode-org:developfrom
Mars-60:fix-poetry-optional-dependency
Open

Fix Poetry lock optional dependency parsing#5295
Mars-60 wants to merge 2 commits into
aboutcode-org:developfrom
Mars-60:fix-poetry-optional-dependency

Conversation

@Mars-60

@Mars-60 Mars-60 commented Aug 30, 2026

Copy link
Copy Markdown

Fixes #5294

Tasks

  • Reviewed contribution guidelines
  • PR is descriptively titled 📑 and links the original issue above 🔗
  • Tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR
    Run tests locally to check for errors.
  • Commits are in uniquely-named feature branch and has no merge conflicts 📁
  • Updated documentation pages (if applicable)
  • Updated CHANGELOG.rst (if applicable)

Description

PoetryLockHandler incorrectly reported resolved Poetry lockfile packages as optional even when the package had optional = false in poetry.lock.

The fix maps Poetry's optional field to ScanCode's is_optional field:

is_optional = package.get("optional", False)

Signed-off-by: Adnan Raza <imadnanraza4@gmail.com>

@AyanSinhaMahapatra AyanSinhaMahapatra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks @Mars-60 see comments for updating the tests, ready to merge otherwise.

expected_loc = self.get_test_loc('pypi/poetry/univers-poetry.lock-expected.json')
self.check_packages_data(package, expected_loc, regen=REGEN_TEST_FIXTURES)

def test_parse_poetry_lock_package_optional(self):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you use an actual test file and expected results file like in the tests above?

Comment thread src/packagedcode/pypi.py
resolved_package = models.PackageData.from_data(package_data, package_only)

is_optional = package.get("is_optional") or True
is_optional = package.get("optional", False)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks, good catch

Comment thread tests/packagedcode/test_pypi.py Outdated
}

assert dependencies_by_purl['pkg:pypi/required-dep@1.0.0']['is_optional'] is False
assert dependencies_by_purl['pkg:pypi/optional-dep@2.0.0']['is_optional'] is True

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need to parse results and assert specifically, we run whole file results test with a test and an expected file, just check the expected behaviour and results as a whole

Signed-off-by: Adnan Raza <imadnanraza4@gmail.com>
Copilot AI lite review requested due to automatic review settings September 2, 2026 14:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 Approval recommended

The fix correctly maps Poetry’s optional field to is_optional and is backed by updated fixtures plus a targeted regression test.

Pull request overview

Fixes PoetryLockHandler so resolved dependencies reflect Poetry’s optional = true/false flag instead of incorrectly defaulting to optional.

Changes:

  • Update PoetryLockHandler.parse() to map Poetry lockfile optional → ScanCode is_optional.
  • Refresh Poetry fixture expected outputs and add a minimal lockfile fixture + test covering both optional and non-optional packages.
  • Document the fix in CHANGELOG.rst and add contributor to AUTHORS.rst.
File summaries
File Description
src/packagedcode/pypi.py Fixes optional parsing by using package.get("optional", False) when creating DependentPackage entries.
tests/packagedcode/test_pypi.py Adds a regression test validating optional vs. required packages in a Poetry lockfile.
tests/packagedcode/data/pypi/poetry/univers-poetry.lock-expected.json Updates expected dependency is_optional values to match optional = false in the fixture lockfile.
tests/packagedcode/data/pypi/poetry/univers-package-assembly-expected.json Updates assembled expected outputs to align with corrected is_optional behavior.
tests/packagedcode/data/pypi/poetry/optional/poetry.lock Adds a minimal Poetry lockfile fixture containing both optional = false and optional = true packages.
tests/packagedcode/data/pypi/poetry/optional-poetry.lock-expected.json Adds expected parsed output for the new optional/required fixture.
CHANGELOG.rst Adds an entry for the Poetry optional dependency parsing fix (links issue #5294).
AUTHORS.rst Adds the contributor.
Review details
  • Files reviewed: 7/8 changed files
  • Comments generated: 0
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@Mars-60

Mars-60 commented Sep 2, 2026

Copy link
Copy Markdown
Author

Updated the test to use an actual Poetry lockfile fixture and expected results file, as suggested. Also removed the explicit assertions and now uses check_packages_data() for the whole result.

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.

PoetryLockHandler incorrectly marks non-optional packages as optional

3 participants