search

sarugaku / resolvelib

Resolve abstract dependencies into concrete ones
ISC License
138 stars 31 forks source link

Awkward wording in docstring for `find_matches()` #137

Closed brettcannon closed 11 months ago

brettcannon commented 12 months ago

The docstring says the identifier parameter "identifies the dependency matches of which should be returned." Is this trying to say it "it identifies the dependency for which matches should be returned"?

uranusjr commented 11 months ago

It’s trying to say (iirc) for each candidate returned by find_matches, identify(candidate) should return the same value as identifier. A rewording would be helpful…

brettcannon commented 11 months ago

Does that mean identify() can be called on a candidate as well? The identify() docstring only mentions requirements:

https://github.com/sarugaku/resolvelib/blob/77b256cdfb747e86112025d75cd698861ce355a5/src/resolvelib/providers.py#L5-L9

The typing suggests it can be either, but the README says " "requirement" ... SHOULD NOT be used when describing a Candidate, to avoid confusion", so that docstring might also be misleading.

And I'm happy to submit a PR to clear up the wording on all of this once I understand exactly what's expected.

uranusjr commented 11 months ago

Yes; the argument is called requirement_or_candidate (and typed as Union[RT, CT]). The docstring does indeed need some fixup.

brettcannon commented 11 months ago

I'll create a PR to try to clarify both docstrings.

brettcannon commented 11 months ago

https://github.com/sarugaku/resolvelib/pull/138 is my attempt to clarify things.