fix to python constraints ignored when markers present #4965

[maciejskorski]

Pull Request Check List

Resolves:

#3444
#4959
#4958
#4965

• Added tests for changed code.
• Updated documentation for changed code.

[dimbleby]

Should this be a fix to the classmethod on poetry-core's Factory?

[maciejskorski]

Should this be a fix to the classmethod on poetry-core's Factory?

@dimbleby I think so! For the guilty lines please have a look to #4965. Two possibilities to fix:
a) either to change create_dependencies in poetry.core (a dependency repo, so more complicated?)
b) apply a patch to the inherited class in poetry (as in this draft? )

[dimbleby]

If the fix belongs in poetry-core, surely just make an MR against that project? I'm not seeing why this is "more complicated"!

[maciejskorski]

@dimbleby @radoering appreciate your feedback.

If the fix belongs in poetry-core, surely just make an MR against that project? I'm not seeing why this is "more complicated"!

Not clear where the fix belongs to. Note that poetry.core.factory exposes a base/template class, which is then rewritten in poetry.factory (BaseFactory->Factory). Isn't legit to fix the problem in the actual class, rather than in its template?

Think of side effects when changes to the template from poetry.core.factory escalate to other descendants, tests included.

Please advise, but don't be too quick. I don't know the code well enough to see there will be no side effects :-)

[dimbleby]

I don't see any upside in having two slightly different implementations for create_dependency(), one of which has a bug and one of which doesn't.

[radoering]

Without having the difference between poetry's Factory class and poetry-core's base Factory class investigated further, what I don't like is the amount of code duplication if the change is made in the sub class. If the change can be made in the base class without importing anything from poetry and has the nature of a fix (instead of a new feature), it probably belongs to poetry-core in my opinion.

As for side effects, I hope they will be revealed by reviews and existing tests.

[neersighted]

Can you please rebase this onto the latest poetry-core release which includes the fixes to make these tests pass?

[miigotu]

I just hit this bug with this constraint:
cryptography = {version = "<35.0.0", markers = "sys_platform == 'linux' and platform_machine in 's390x, ppc64le,'", python = "<3.8"}

[neersighted]

Hi @miigotu -- this PR is stale and the issues it set out to address have been solved in a successor PR against poetry-core. If you are running into an issue you believe is related, please open a new bug report with a pyproject.toml, and detailed comparisons of what Poetry is doing vs. what you expect it to be doing.

Thanks!

[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.