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

Integrate Twisted's customizations #315

Closed
mthuurne opened this issue Dec 4, 2020 · 12 comments
Closed

Integrate Twisted's customizations #315

mthuurne opened this issue Dec 4, 2020 · 12 comments

Comments

@mthuurne
Copy link
Contributor

mthuurne commented Dec 4, 2020

Twisted has a source file in which it customizes some aspects of pydoctor:

  • workaround for doc URL lookups from zope.interface
  • formatting of deprecation notices
  • make test packages invisible, except twisted.test.proto_helpers

I'm not sure if the workaround is still necessary, but if it is, it would be useful for all projects that want to link to zope.interface documentation, which means it would be better to move this code into pydoctor.

The decorators used to mark deprecations in Twisted are public API, so a project that uses Twisted could use those as well. In that case, it would be an advantage if the functionality for formatting them was inside pydoctor. In the future, we might want to support other deprecation mechanisms such as regret, which would be easier to do in a consistent fashion if all the code is inside pydoctor.

Having the ability to exclude certain packages from the documentation sounds like a useful feature, so we could consider to facilitate that through command line options instead of customization code.

Put together, I think all of the code customizations that Twisted currently does would actually fit better inside pydoctor itself. Integrating them would have multiple advantages:

  • for users: it makes pydoctor more capable out of the box
  • for pydoctor developers: we can change the design of pydoctor without worrying about breaking Twisted's docs
  • for Twisted developers: better chances of this code being kept up-to-date

The template customizations would remain in Twisted. We are working on breaking up the templates into smaller ones, to make customizations more fine-grained, see #299 for details.

@mthuurne
Copy link
Contributor Author

mthuurne commented Dec 4, 2020

Note that the code for deprecation notices is currently spread over both Twisted and pydoctor: Twisted extracts them, but pydoctor inserts them into the output and filters the deprecation decorators out of the output.

@tristanlatr
Copy link
Contributor

tristanlatr commented Dec 4, 2020

Having the ability to exclude certain packages from the documentation sounds like a useful feature, so we could consider to facilitate that through command line options instead of customization code.

May be :public:, :private: epydoc/rst tags ?

Epydoc had also an exclude option, which is also a good idea

@mthuurne
Copy link
Contributor Author

mthuurne commented Dec 4, 2020

Having the ability to exclude certain packages from the documentation sounds like a useful feature, so we could consider to facilitate that through command line options instead of customization code.

May be :public:, :private: epydoc/rst tags ?

Good idea. Especially for complex cases, where you first want to exclude something (twisted.test) and then re-include a subset (twisted.test.proto_helpers), tagging the packages might be more elegant than command line options.

And we already had a tag idea for marking private __init__ methods.

Epydoc had also an exclude option, which is also a good idea

An advantage of options is that they show up in --help, which makes them easier to discover than tags.

Maybe we could do both? Although I would only want to do that if there are specific scenarios in which one approach is better than the other, not just to avoid having to make a decision.

@tristanlatr
Copy link
Contributor

Both make sens. --exclude option might be useful to exclude whole packages or class while :private: and :public: can be added on a per method basis.

@adiroiban
Copy link
Member

@mthuurne thanks for the regret link. It looks like nice project.

In twisted we always had to work with various bugs for deprecation helpers... so I think that if is worth adopting regret in Twisted so that we don't have to reinvent the wheel.

But regret has still limited support, without support for attribute deprecations...but I think that it is worth contributing to regret

@mthuurne
Copy link
Contributor Author

mthuurne commented Dec 6, 2020

Note that Twisted marks modules named "test" as hidden rather than private. Hidden means they are completely excluded from the output, while private means they are invisible by default, but can be revealed via the "Show Private API" button.

Tests in Twisted are documented, so I think it's a pity to omit that documentation from the generated HTML. It also means that some of pydoctor's sanity checks on docstrings will not run, making it more likely for mistakes in test documentation to remain undetected.

@mthuurne
Copy link
Contributor Author

One advantage of an exclude option at the pydoctor level would be that we could skip the parsing and analysis of the excluded code altogether, while marking modules as hidden means they are analyzed and then not included in the output.

@mwhudson
Copy link
Contributor

mwhudson commented Jan 4, 2021 via email

@mthuurne
Copy link
Contributor Author

mthuurne commented Jan 4, 2021

I'm not sure I have 100% context here, but one problem with that is that sometimes objects are imported from private modules into public ones and if you don't parse the private modules then you can't document objects like this. This is unlikely to be an issue for twisted's tests, of course.

There are currently 3 visibility levels for documentable objects:

  • public: shown by default
  • private: shown after clicking the "show private API" button
  • hidden: not rendered to HTML

Excluding modules would mean we skip those files when iterating over all modules in a package, so that goes a step further than "hidden".

I don't think we need to change how "public" and "private" work. So re-exporting definitions from private modules to public modules would continue to work as it does now.

What the custom System class of Twisted does is mark all test modules as "hidden" and I have some doubts about whether that is the best approach. If we'd mark the tests as "private" instead of "hidden", the test docs would be available in HTML, after clicking "show private API". Alternatively, if we'd exclude the tests from processing, we would speed up documentation generation for Twisted.

My first thought was that if we have docstrings in test modules, we should make them available in the HTML. But after some more thought, including discussion with @adiroiban in #341, I don't actually think it's very likely that people will be reading test docstrings in HTML: docstrings for tests are useful when a test fails or when maintaining the test suite and in both those situations the person interested in the docstrings will already have the test code opened in a text editor.

I'm not sure we even need "hidden" as a visibility level: currently there is no situation which in pydoctor itself marks an object as hidden: the custom System class of Twisted is the only code that does.

@tristanlatr
Copy link
Contributor

I'm not sure we even need "hidden" as a visibility level: currently there is no situation which in pydoctor itself marks an object as hidden: the custom System class of Twisted is the only code that does.

I like the hidden privacy class value. Without the Twisted hack that makes all test modules hidden the search index is significantly smaller that with all test modules.

@tristanlatr
Copy link
Contributor

tristanlatr commented Apr 30, 2022

Privacy tweaking is now part of pydoctor (with option --privacy).

There still the following todo:

  • workaround for doc URL lookups from zope.interface
  • formatting of deprecation notices

I'll work on the later point. I'll try to refactor such that generic extra informations to be added to objects, like deprecation, but other kind of information could be added to a "extra informations" list.

The code is here: https://github.com/twisted/twisted/blob/trunk/src/twisted/python/_pydoctor.py
The tests are here: https://github.com/twisted/twisted/blob/trunk/src/twisted/python/test/test_pydoctor.py as well as here: https://github.com/twisted/twisted/blob/3bbe558df65181ed455b0c5cc609c0131d68d265/src/twisted/python/test/test_release.py#L516
And the templates: https://github.com/twisted/twisted/tree/trunk/src/twisted/python/_pydoctortemplates

@tristanlatr
Copy link
Contributor

I think this issue can closed once #554 is merged, the intersphinx customizations are tracked in #384.

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

No branches or pull requests

4 participants