#203 Incomplete fix for __import__

closed-accepted
nobody
None
5
2012-08-26
2012-08-23
Toshio Kuratomi
No

Thanks for the quick fix to https://sourceforge.net/tracker/?func=detail&aid=3559988&group_id=38414&atid=422030 but one of the things that I mentioned there is that this problem will affect all the code touched by svn revision 7486. The files changed to use __import__ with level=1 in that revision are:

docutils/parsers/__init__.py
docutils/parsers/rst/directives/__init__.py
docutils/parsers/rst/languages/__init__.py
docutils/writers/__init__.py
docutils/readers/__init__.py
docutils/languages/__init__.py

writers has now been fixed but the others will exhibit the problem behaviour -- they will load modules located in the docutilstree but they won't be bale to load modules created locally by the user for a particular build.

Discussion

  • is this a real problem, local readers, llanguages, parsers ?
    directives might be.

    need some time to prepare tests (failing) and fix them

     
  • I'm not touching any code that is failing with these at the moment but that doesn't mean it isn't out there (I wasn't aware of get_writers_class until the mercurial devs let me know their build was now failing).

    * Readers has a public get_reader_class so third parties could be using it for local writers the same way as get_writer_class
    * language (both the toplevel and in parsers) I don't know. When I first looked at it, I was inclined to think that it was an implementation detail (for instance, get_language() is not mentioned in any documentation). OTOH, a site that wants to produce documentation in their native language that isn't in docutils may very well be taking the easy way out and simply have a local LANG.py file.
    * parsers I'm also not sure about. My first thought was that people using docutils are likely also using rst so alternate parsers shouldn't be that common. However, __import__ is used in a public get_parser_class() like get_writer_class(). The documentation (pep-0258.txt ) even says: "Alternate markup parsers may be added." I suppose that someone who is in the process of converting from other formats to rst might have alternate parsers so they could use the docutils framework to work with their legacy formats.
    * directives you've already mentioned

    So... I can think of reasons people might be either using or abusing the API that calls __import__ for all of these. Since I'd rather not get bug reports and have to push new packages one-at-a-time my inclination would be to fix all these potential areas now.

     
  • add test cases for parsers, readers, and languages

     
  • Fix parsers, readers, and languages to pass the new tests

     
  • I've just uploaded a patch with testcases and a patch with fixes for readers, parsers, and languages. I looked at directives but haven't touched it yet. The code is more complex there so I need to digest what's going on a little more.

     
  • Okay, my reading of the directives code and documentation is that the docutils.parsers.rst.directives.directive() function (where the __import__ is used) is only used by in-tree modules. Out-of-tree directives use docutils.parsers.rst.directives.register_directive() to add an already imported class to the valid directives. So there's no need to patch the __import__ in directives() since extending this functionality will be done via a different set of code.

     
    • status: open --> closed-accepted