Opened 5 years ago

Closed 5 years ago

#15233 closed (fixed)

Docs misidentify modules for classes/functions

Reported by: Aryeh Leib Taurog <vim@…> Owned by: gabrielhurley
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@…> 5 years ago.
patch for documentation module paths
modulecheck.py (810 bytes) - added by Aryeh Leib Taurog <vim@…> 5 years ago.
script to check module paths of objects in documentation index

Download all attachments as: .zip

Change History (11)

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

patch for documentation module paths

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

script to check module paths of objects in documentation index

comment:1 Changed 5 years ago by russellm

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 5 years ago by gabrielhurley

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 5 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 5 years ago by gabrielhurley

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 5 years ago by gabrielhurley

  • Resolution set to fixed
  • Status changed from new to closed

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 5 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 5 years ago by Aryeh Leib Taurog <vim@…>

  • Resolution fixed deleted
  • Status changed from closed to reopened

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 5 years ago by gabrielhurley

  • Owner changed from nobody to gabrielhurley
  • Status changed from reopened to new

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 5 years ago by gabrielhurley

  • Resolution set to fixed
  • Status changed from new to closed

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