Re: [Docutils-users] Import bug in docutils/parsers/rst/state.py

[David G. <go...@py...>]

On Thu, Mar 21, 2013 at 3:10 AM, Guenter Milde <mi...@us...> wrote:
> On 2013-03-18, Larry Hastings wrote:
>
>> The current version of docutils/parsers/rst/state.py has this code:
>
>>     try:
>>          import roman
>>     except ImportError:
>>          import docutils.utils.roman as roman
>
>> Since there is no "roman.py" in "docutils/parsers/rst", I'm not sure
>> when that bare "import roman" is a good idea.  It actually seems to me
>> like a bad idea, because if you have a "roman.py" in the current
>> directory, it'll find that instead.  (And, if that happens to be a
>> Python 2 file, while you're running Python 3, you get a SyntaxError.)
>> Alternatively, if 'roman' in sys.modules, "import roman" will grab
>> that... whatever it is.
>
>> Surely it would be best to drop the first three lines of this snippet,
>> and always explicitly import docutils.utils.roman as roman.  After all,
>> explicit is better than implicit.
>
> "roman" is a 3rd-party Python module.
>
> The idea with this implementation is to use the upstream module if it is
> installed, as it may be a newer version with bugfixes.
>
> As "roman" is not in the standard library, we cannot be sure it's installed,
> therefore we include/use a (possibly outdated) copy as fallback.
>
> @David: do you have a preferred strategy regarding "roman"?

I don't think the roman.py that Docutils uses has been changed in
years. Even if it is changed, the changes may *break* Docutils rather
than help it. If there are beneficial changes, we can incorporate them
into docutils.utils.roman in due course. If there are bugs, that's
what the tracker is for.

I agree with Larry: drop the conditional import. Let's keep it simple.
And while we're at it, replace with a simpler import (no redundant
"roman")::

    from docutils.utils import roman

-- 
David Goodger <http://python.net/~goodger>