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

Fix renumbering references in tables #131

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

Conversation

peterjc
Copy link

@peterjc peterjc commented Oct 25, 2017

Proposed fix for #130, and tested on that example only.

Query: Does the rename function never make the reference shorter? If not, my code can be simplified.

@jnothman
Copy link
Member

Won't work if the table isn't wide enough.

This reference renaming stuff is a mess. I wonder if we need to rework it altogether.

@peterjc
Copy link
Author

peterjc commented Oct 25, 2017

I'd noticed this could break if the table wasn't wide enough (and making the table wider is really too much work and too fragile too attempt), but strangely when I tried that as a test case it still seemed to work...

@jnothman
Copy link
Member

The table width thing might depend on which column it's in...? Either way, it wouldn't work for simple tables, I think.

@pv
Copy link
Member

pv commented Oct 25, 2017

I feel the reference renumbering / footnote translation perhaps should operate on doctree level not rst source. Walk the tree and transform footnotes/footnote references to citations if necessary --- a bit more complex to write (iirc docutils is still mostly undocumented).

@jnothman
Copy link
Member

I tried looking at appropriate events to hook into yesterday. There's nothing in autodoc that hooks in at doctree time, so you can't do it once per docstring, and would need some other way of detecting which units are something numpydoc has processed. Sphinx has doctree-read and doctree-resolved. The latter seems inapplicable since "all references have been resolved". The former... might help, but I don't really understand where "Emitted when a doctree has been parsed and read by the environment, and is about to be pickled." places it with respect to resolution.

@peterjc
Copy link
Author

peterjc commented Oct 26, 2017

I fully agree that my pull request is far from general. It should work in many cases, and allows the user to manually expand the table by adding a column of spaces if needed as a workaround, but is at best a stop-gap measure.

I don't know enough about the architecture but agree processing the doctree if possible seems a better fix for #130.

@jnothman
Copy link
Member

jnothman commented Nov 1, 2017

I tried to implement a generic doctree processing-based solution, but it proved a lot of work. #136 exacerbates the problem by making references 13 chars longer. #136 currently adds advice to the user not to put references in tables. I think this should be the status for the next release. The alternatives are:

  • a doctree based solution, but this involves handling footnote_reference, citation_reference, pending_xref and problematic nodes, as well as rewiring the reference-target mapping in the document object.
  • identifying the tables and widening them. This may be hard to test for rowspans and colspans, etc.

Base automatically changed from master to main March 5, 2021 12:29
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

Successfully merging this pull request may close these issues.

3 participants