find_distributions documentation issues

[jaraco]

In GitLab by @mhsmith on Jun 1, 2020, 22:33

The section "Extending the search algorithm" says that find_distributions must return an iterable. However, the current implementation actually requires an iteratOR, because it calls next here. So if someone writes a find_distributions which returns a list, this will fail.

Separately, the DistributionFinder.Context .name attribute is documented as indicating "names to match" (plural). However, it's actually just a single string.

Finally, it says that DistributionFinder.Context "may supply other relevant context". This is so vague as to be useless. If it's supposed to mean "we may add other properties in the future", isn't that true of any API?

[jaraco]

In GitLab by @jaraco on Jun 5, 2020, 18:12

mentioned in commit 111df08

[jaraco]

In GitLab by @jaraco on Jun 5, 2020, 18:17

Good feedback. Thanks for registering the issues.

the current implementation actually requires an iterator.

Good point. I'm not sure which should change, the documentation or the implementation. I'm leaning toward the implementation should support any iterable (and thus should invoke iter() on the result before calling next()). Thoughts?

Separately, the DistributionFinder.Context .name attribute is documented as indicating "names to match" (plural). However, it's actually just a single string.

Fixed in 111df08.

Finally, it says that DistributionFinder.Context "may supply other relevant context". This is so vague as to be useless. If it's supposed to mean "we may add other properties in the future", isn't that true of any API?

Context accepts arbitrary keywords. So if one's custom finder's find_distribution arguments could select on some other factor and one's clients were willing to supply that factor, the Context object can relay those factors. It's meant to be a documented feature so that those extending the search would have confidence they could rely on that interface to solicit and consume additional context without any changes to importlib metadata. That is, the Context is explicitly implementer extensible.

[jaraco]

In GitLab by @jaraco on Jun 5, 2020, 18:51

closed via commit 23ff3cb

[jaraco]

In GitLab by @jaraco on Jun 5, 2020, 19:00

mentioned in commit 23ff3cb

[jaraco]

In GitLab by @mhsmith on Jun 7, 2020, 13:15

I'm leaning toward the implementation should support any iterable

I agree.

Context accepts arbitrary keywords. So if one's custom finder's find_distribution arguments could select on some other factor and one's clients were willing to supply that factor, the Context object can relay those factors.

OK, I see this is explained in the docstring, but the online documentation doesn't define the Context constructor at all.

It's also unclear what .path and .name will be if you don't pass them to the Context constructor (None and sys.path respectively). Again, this is answered in the docstrings but not in the online documentation.

[jaraco]

In GitLab by @jaraco on Jun 7, 2020, 14:54

This is answered in the docstrings but not in the online documentation.

In #119, Pradyun has requested that API docs (docstrings and code-generated documentation) be included in the documentation. I think that's a fine idea and would limit the problem of having the documentation in multiple places with possibly out-of-sync descriptions. If you're willing to help with that effort, it would likely address both concerns.