Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add warning about the global set_element_class_lookup #341

Merged
merged 3 commits into from
Mar 13, 2022
Merged

Add warning about the global set_element_class_lookup #341

merged 3 commits into from
Mar 13, 2022

Conversation

xmo-odoo
Copy link
Contributor

It's a bit of an attractive nuisance, as it basically turns off parser-specific class lookups, which can be surprising to people who have not looked up the mechanism in details (it certainly surprised me).

Also set inherited-members in the autodoc config: it's a bit dodgy but because _BaseParser is @cpython.internal it looks like autodoc can't see it at all, and so anything defined on _BaseParser is invisible in the new rst-based doc.

Could not test the docs because I'm hitting cython/cython#3876, does one of the statuses provide a doc to look at?

It's a bit of an attractive nuisance, as it basically turns off
parser-specific class lookups.
@xmo-odoo
Copy link
Contributor Author

Also the new doc is... a bit of a mess. Would it be OK to submit PRs to commit the apidoc-generated stuff and start hand-crafting the documents in order to make them more logical and stuff? e.g. the lxml.etree module starts with 6 pages of exceptions which doesn't make for the most nicest of reads.

@scoder
Copy link
Member

scoder commented Mar 11, 2022

Could not test the docs because I'm hitting cython/cython#3876,

That's long solved, though. That was a problem in 0.29.22, even the legacy stable release of Cython is at 0.29.28 now.

src/lxml/classlookup.pxi Outdated Show resolved Hide resolved
@scoder
Copy link
Member

scoder commented Mar 11, 2022

Also the new doc is... a bit of a mess. Would it be OK to submit PRs to commit the apidoc-generated stuff and start hand-crafting the documents in order to make them more logical and stuff? e.g. the lxml.etree module starts with 6 pages of exceptions which doesn't make for the most nicest of reads.

Don't know if there is a way in apidoc to change the ordering, but there probably is. I agree that exceptions certainly aren't the most interesting part. Alphabetic ordering is helpful, though, so maybe the normal classes should just go first, followed by functions, followed by exceptions.

Also lxml.etree, lxml.html and lxml.objectify should rather be shown at the top of the list, in that order.

@xmo-odoo
Copy link
Contributor Author

Also the new doc is... a bit of a mess. Would it be OK to submit PRs to commit the apidoc-generated stuff and start hand-crafting the documents in order to make them more logical and stuff? e.g. the lxml.etree module starts with 6 pages of exceptions which doesn't make for the most nicest of reads.

Don't know if there is a way in apidoc to change the ordering, but there probably is. I agree that exceptions certainly aren't the most interesting part.

Sadly I don't think there's a way for apidoc to do that, hence the query about committing the files it generates, instead of running apidoc on doc build it'd just be used once to generate the "initial" state (as a scaffolding tool), then those would be edited directly.

The code being committed can still use autodoc though.

@xmo-odoo
Copy link
Contributor Author

Thinking about it further, maybe it would (also?) be a good idea to clarify that the global element lookup is really the first step for resolution? And remove the word "default" from the docstring of lxml.etree.set_element_class_lookup?

That kinda gives the impression that it is the fallback, but it's the other way around right? When an element proxy needs to be created, lxml invokes _elementFactory, which calls LOOKUP_ELEMENT_CLASS, which is the object set by `set_element_class_lookup. So that is the first step, and (optionally) calling the parser's lookup' (or not) is its responsibility.

@scoder
Copy link
Member

scoder commented Mar 13, 2022

I'd rather not commit generated files. They just bloat the history, complicate merges and are annoying to work with in various ways. The way to control API documentation in Sphinx seems to be module files and/or TOC files.

There are also various hacks that people have come up with.

I'm not particularly tied to sphinx-apidoc. If there's a different solution, I'm open.

src/lxml/classlookup.pxi Outdated Show resolved Hide resolved
@scoder scoder merged commit e983807 into lxml:master Mar 13, 2022
@scoder
Copy link
Member

scoder commented Mar 13, 2022

Thanks

@xmo-odoo
Copy link
Contributor Author

Haha nah thank you, the final version is quite a bit better than the original proposal. Cheers.

@xmo-odoo xmo-odoo deleted the global-set-element-warn branch March 14, 2022 06:38
@xmo-odoo
Copy link
Contributor Author

I'd rather not commit generated files. They just bloat the history, complicate merges and are annoying to work with in various ways. The way to control API documentation in Sphinx seems to be module files and/or TOC files.

There are also various hacks that people have come up with.

I'm not particularly tied to sphinx-apidoc. If there's a different solution, I'm open.

Committing generated files is an issue when regularly re-exporting isn't it? Here the output of apidoc would provide a skeleton / starting point, then it wouldn't be used anymore, so there's no more complication than if the initial state had been written by hand. That's how I've used it in the past anyway. Pretty much what the second answer suggests in your link.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants