Skip to content

transforms module api changes in the documentation #11572

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fredrik-1
Copy link
Contributor

@fredrik-1 fredrik-1 commented Jul 4, 2018

I thought that it was very difficult to get an oversight of the transforms module in the documentation so I propose a change.

I wrote a new autosummary template autoclass.rst and used it for rendering the transforms module.

I think that it result in a much easier to read documentation without the need of much manual work.

edit: I don't understand why I get a warning /home/circleci/project/doc/api/_as_gen/matplotlib.transforms.Affine2D.rst:2:Literal block expected; none found.

I have tested many variants of the template. The resulting files look good to me and the html files looks and works as expected when I build them on my computer (with the same warning).

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@fredrik-1 fredrik-1 force-pushed the transforms_api branch 8 times, most recently from 1d03a8f to 095cb54 Compare July 5, 2018 07:24
@dstansby
Copy link
Member

dstansby commented Jul 5, 2018

This kind of looks like a manual way of doing what the sphinx automodapi extension does: http://sphinx-automodapi.readthedocs.io/en/latest/automodapi.html , which is automatically list all the classes and then the functions. Would using this extension achieve what you want @fredrik-1 ?

@fredrik-1
Copy link
Contributor Author

I finally find the problem. Some of the methods docstrings had a first line that ended with ::.

Here is the new transform module page.

I am going to edit the method docstrings in a better way later.

@tacaswell tacaswell added this to the v3.0 milestone Jul 6, 2018
@tacaswell
Copy link
Member

Does this impact other pages? If it does not should we be using this else where?

attn @ImportanceOfBeingErnest @anntzer @timhoffm

@fredrik-1
Copy link
Contributor Author

No, it doesn't impact any other pages at the moment. I think that it should be used on modules with many classes/functions or when the classes has many methods, like for example the figure module.

a c e
b d f
0 0 1
2D affine transformations are performed using a 3x3 numpy
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The info that the last line is 0, 0, 1 seems worth leaving in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am going to look through those changes later. I just wanted a working example when I found out what cases the warnings.

@anntzer
Copy link
Contributor

anntzer commented Jul 6, 2018

Looks good and I guess may be worth making this template the default and opt-out on a case by case basis if needed.
I don't know anything about automodapi but I agree it would be nice not to reinvent the wheel if such features already exist elsewhere -- at least we should consider it.

@ImportanceOfBeingErnest
Copy link
Member

Let me first say that I would really support making use of the automodapi extension. Writing the toc-trees by hand is somehow contradictory to automated documentation. Also it seems that there is currently already some option like this activated for content generated by the automodule.rst template. (E.g. in the axis_grid1 docs) I'm just not sure how to use this here, where the rst file is not completely autogenerated.

The problem of :: in the first line is not too clear to me, at least the rst-docs do not meantion that it would not be allowed. In any case, if the fully minimized form

Paragraph::

    Literal block

is not possible, I would prefer to use the Expanded form:

Paragraph:

::

    Literal block

and not artificially introduce new paragraphs like

Paragraph.

New paragraph just to make mimimal form work::

    Literal block

But maybe I'm missing something here?


Does this impact other pages?

If this is really a problem, it does at least not accur in any of the cases where an autoclass-type template is in use, simply because otherwise we would see such errors popping up. But I can imagine that such cases are present in docstrings that are currently produced via automodule.

@fredrik-1
Copy link
Contributor Author

I don't know the details about automodapi but I believe that this template is very similar to sphinx default class template with the difference that attributes are not included and that the examples get automatically linked in this one.

@fredrik-1
Copy link
Contributor Author

fredrik-1 commented Jul 6, 2018

The problem with :: in the first line in a method was that autosummary reads the first line of a method and write it on the summary page. This resulted in a warning (that I didn't understand) when a docstring where something like

A matrix::
   a b
   c d

@fredrik-1
Copy link
Contributor Author

I have been thinking about starting a discussion about how to render the API but I wanted this to work before I did that.

I believe that completely auto generated module pages are sometimes good and sometimes not.

I think that modules with many classes and/or functions need some manual editing (or special flags in the module files) to order the classes and functions in some way.

automodapi might result in nice automatic pages but I don't think that automodule, autosummary and templates can render completely automated pages well enough for large modules.

@fredrik-1
Copy link
Contributor Author

The questions are something like:

  • How and when to split the documentation into smaller parts.
    • The whole module in one page
    • A whole class on one page
    • Every function or method on its own page
  • Automatically or manual rendering the lists of classes, functions and methods?
    • using python scripts for writing rst pages?
  • Extra explaining text on some of the pages?

@ImportanceOfBeingErnest
Copy link
Member

So does the Expanded form work? It should, as far as I can see.

If we stick to this specific case of the transforms docs, you would not want to actually hardcode all classes and methods, but rather have that list autogenerated. Once you found out how to do that, the same method can be applied to many other pages (not all, for sure, e.g. axes will always need to be manually generated).

@fredrik-1
Copy link
Contributor Author

What do you mean with the expanded form?

The solution in this PR hardcode the classes and nothing more. The advantage of this is that you can change the order of the classes or put them in sections. The order here is actually the one in transforms.py.

@ImportanceOfBeingErnest
Copy link
Member

What do you mean with the expanded form?

See my previous post.

The solution in this PR hardcode the classes and nothing more.

Exactly. I'm questioning that hardcoding is the best solution here and would prefer an automated solution, as would e.g. be possible with automodapi or maybe even with the currently available extentions - this needs to be found out.

@fredrik-1
Copy link
Contributor Author

Also it seems that there is currently already some option like this activated for content generated by the automodule.rst template. (E.g. in the axis_grid1 docs) I'm just not sure how to use this here, where the rst file is not completely autogenerated.

The template automodule.rst, autofunctions.rst and autoclass.rst more or less does the same things on different objects. One drawback with them are that they work with the autosummary directive so that something needs to be hard coded to actually let the scripts work and the autogenerated stuff get on a new page.

This is not really a problem in for example the axis_grid1 case where there are several modules that can be easily written down and the page hierarchy generated seems natural. It is really not that good though if only one module is going to be generated. This is the case for pyplot where you get one extra level of page hierarchy that is unnecessary.

I don't think that it is possible to use the scripts without using the autosummary directive.

I believe that you easily can get a good result with the templates that exist now and it is possibly to make a new template slightly differently to fit a special module better (the automodule is actually written for axes_grid and the autofunction for pyplot). It is easy to do the manual work and the result is often better with some easy to do editing.

One method I think should work for a completely automatic rendering is to write a .rst file in the api folder that make an autosummary, with automodule (that use autoclass.rst) of one, or several modules and then link the files from _as_gen in the index.rst file. That would be automatic but you miss the opportunity to write anything on the module pages that are not in the module.py file.

@ImportanceOfBeingErnest
Copy link
Member

Seems like this brings us back to the start: One should try out the automodapi. At least theoretically it looks like the best way to automatically get the desired doc format and at the same time allow for custom content.

@fredrik-1
Copy link
Contributor Author

I have tried automodapi, the result can be seen in #11595, if it can be used on circleci.

The result looks good but I am note sure if it is possible to include the links to the examples without same hacking.

@tacaswell
Copy link
Member

I am going to re-milestone this as 3.1 (in the interest of getting the 'must get done' list down to something reasonable), but it can go in when ever it is ready.

@tacaswell tacaswell removed this from the v3.0 milestone Jul 8, 2018
@tacaswell tacaswell added this to the v3.1 milestone Jul 8, 2018
@tacaswell
Copy link
Member

I just want to say that the work that both @fredrik-1 and @ImportanceOfBeingErnest are doing here is important, hard, and appreciated!

@fredrik-1
Copy link
Contributor Author

I edited the transform.py file and made a small change in automodule.py that don't change the output.

Here is the result.

I think this is a good solution for rendering a module. It might be possible to get the same result automatically with automodsumm in automodapi in the future. I have a solution that work for me but I don't know if and when it can be merged into automodapi.

@jklymak
Copy link
Member

jklymak commented Sep 10, 2020

@timhoffm this could use your attention...

@jklymak jklymak marked this pull request as draft September 10, 2020 15:10
@QuLogic QuLogic modified the milestones: v3.4.0, v3.5.0 Jan 21, 2021
@QuLogic QuLogic modified the milestones: v3.5.0, v3.6.0 Aug 23, 2021
@timhoffm timhoffm modified the milestones: v3.6.0, unassigned Apr 30, 2022
@story645 story645 modified the milestones: unassigned, needs sorting Oct 6, 2022
@github-actions
Copy link

github-actions bot commented May 9, 2023

Since this Pull Request has not been updated in 60 days, it has been marked "inactive." This does not mean that it will be closed, though it may be moved to a "Draft" state. This helps maintainers prioritize their reviewing efforts. You can pick the PR back up anytime - please ping us if you need a review or guidance to move the PR forward! If you do not plan on continuing the work, please let us know so that we can either find someone to take the PR over, or close it.

@github-actions github-actions bot added the status: inactive Marked by the “Stale” Github Action label May 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation status: inactive Marked by the “Stale” Github Action status: needs rebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants