Opened 6 years ago

Closed 6 years ago

#15233 closed (fixed)

Docs misidentify modules for classes/functions

Reported by: Aryeh Leib Taurog <vim@…> Owned by: Gabriel Hurley
Component: Documentation Version: master
Severity: Keywords: module directives index
Cc: vim@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Looking at the documentation index, I see: "reverse() (in module django.conf.urls.defaults)," but it isn't there, it's in django.core.urlresolvers. The problem is misplacement/omission/errors in the sphinx directives which identify the module/class of the objects begin documented.

I wrote a simple script to check the genindex.html file written by the html builder. It tries to import all the classes and module-level functions it finds and prints a list of the imports that fail. It's not foolproof, because some of the documented objects aren't really importable.

I'm attaching a patch for the docs; I went through many of the errors reported by said script and tried to tweak the module, currentmodule, function, and method directives so that the documented objects will be documented with correct paths.

Attachments (2)

adjusted-module-directives.patch (28.9 KB) - added by Aryeh Leib Taurog <vim@…> 6 years ago.
patch for documentation module paths
modulecheck.py (810 bytes) - added by Aryeh Leib Taurog <vim@…> 6 years ago.
script to check module paths of objects in documentation index

Download all attachments as: .zip

Change History (11)

Changed 6 years ago by Aryeh Leib Taurog <vim@…>

patch for documentation module paths

Changed 6 years ago by Aryeh Leib Taurog <vim@…>

Attachment: modulecheck.py added

script to check module paths of objects in documentation index

comment:1 Changed 6 years ago by Russell Keith-Magee

Triage Stage: UnreviewedAccepted

comment:2 Changed 6 years ago by Gabriel Hurley

Hi Aryeh. Your script is an interesting little tool, but getting this patch committed is going to require some more work. Let me try and explain the main issues:

  1. This patch is very broad (corrections in over a dozen unrelated files) which makes it a significant job to try and verify that all the changes are correct and don't cause unintended side-effects elsewhere.
  1. The correct solution isn't always to simply prepend the "correct" import path. In some cases what's really needed is a missing module directive, or better indentation under a class directive. Adding that module directive (as you did in some places) can also have unintended consequences of nesting, etc. All this goes back to point 1, that this patch is difficult to verify.
  1. Some of the places where you've added the leading portions of the import paths had them intentionally chopped off to improve readability, so prepending with a "~" is necessary if adding the whole path is the right solution.

My suggestion would be to break this ticket out into smaller tickets that are easier to work with. For example, the changes you've got in there for the File Upload documentation are very useful reST formatting corrections which are excellent, but only marginally related to correct import paths.

Other people have had great luck getting tickets committed for similar types of issues by working on one file or topic at a time. You can see an example of how these kinds of tickets have been broken up successfully by another contributor with this query.

I really appreciate your thoroughness in working through all these, and I want to see the work you've done get incorporated. You just have to make it easy for everyone else, too. ;-)

comment:3 Changed 6 years ago by Aryeh Leib Taurog <vim@…>

Agreed on all points. Will try to follow your advice as time permits. Please set status appropriately.

comment:4 Changed 6 years ago by Gabriel Hurley

I've done an audit on most of the patch provided, and will commit the pieces that I could easily verify and correct so that this work doesn't go to waste and this ticket can be resolved. Please open new tickets for specific areas you find in the future.

comment:5 Changed 6 years ago by Gabriel Hurley

Resolution: fixed
Status: newclosed

In [15549]:

Fixed #15233 -- reST/Sphinx markup corrections in numerous areas, mostly centering around bad crossref targets. Thanks to Aryeh Leib Taurog for the draft patch.

comment:6 Changed 6 years ago by Aryeh Leib Taurog <vim@…>

Wow! Thanks so much. My apologies for not following up sooner; it's just been a busy week.

comment:7 Changed 6 years ago by Aryeh Leib Taurog <vim@…>

Resolution: fixed
Status: closedreopened

Hm. Now I get the following error when I build the documents:

/home/altaurog/djangodoc/trunk/docs/topics/http/urls.txt:3: (SEVERE/4) Duplicate ID: "module-django.core.urlresolvers".

I don't know why I didn't see this earlier. I did compile the docs before submitting the patch. Perhaps it was a different version of sphinx which is less picky. Anyway, I think changing the second module directive to currentmodule would solve this problem.

comment:8 Changed 6 years ago by Gabriel Hurley

Owner: changed from nobody to Gabriel Hurley
Status: reopenednew

Darn, that's what I get for not keeping my copy of Sphinx up-to-date. That error wasn't reported in previous versions, but now that I've got 1.0.7 I'm seeing it. I'll fix that momentarily.

comment:9 Changed 6 years ago by Gabriel Hurley

Resolution: fixed
Status: newclosed

In [15561]:

Fixed #15233 -- Corrected a duplicate module directive that raised a warning on newer versions of Sphinx. Thanks to Aryeh Leib Taurog for the report.

Note: See TracTickets for help on using tickets.
Back to Top