This is probably one example of many where pickling is only tested with _pickle module if it is available rather than separately with C and Python implementation.  I am attaching a patch which implements one possible testing strategy.  If this is acceptable, I think the machinery of retrieving multiple module implementations should be moved to test.support and reused in other test modules.

It looks good on the principle. As you say, factoring out this kind of machinery in test.support is probably the way to go.

In the new patch, issue9104.diff, I factored out import machinery into test.support.import_module_implementations and added it to a a couple of other test modules.  I did not attempt to improve all pickle tests.

The availability of import_module_implementations raises an interesting question of what to do in cases like heapq where one can potentially test 4 combinations of heapq/_heapq with pickle/_pickle.  Given that currently pickling of heapq objects is either not supported or not tested, we don't need to worry about it, but as parallel optimized/pure python module implementations proliferate, we may face exponential explosion of potential test cases.

The code for import_module_implementations seems a bit fragile. Prefixing the module name with an underscore might not always yield the name of the optimized implementation.  An explicit dictionary to map the Python and C implementations may be a better approach.

Otherwise, the code looks good. Feel free to commit it.

> An explicit dictionary to map the Python and C implementations may be a 
> better approach.

+1.

> An explicit dictionary to map the Python and C implementations may be a 
> better approach.


Do you mean a global 

optimized_module = {'pickle': '_pickle'}

in test/support.py?


I don't think I like this idea.  Even with an explicit dictionary like that,  support.import_module_implementations() will not support all possible ways that alternative implementations of a given module may be provided in cpython.  It works for pickle/_pickle and heapq/_heapq, but won't work for io/_io/_pyio.  I would like import_module_implementations() to promote uniformity in the way parallel pure/native modules are designed.  Test suites for modules that want more flexibility can always call import_fresh_module() directly.

I would like to commit this as written.  If a better mechanism for associating native implementation with a pure python module is found, it can easily be added in the future.  Any objections?  The patch only adds more test cases, no code is changed.

> It works for pickle/_pickle and heapq/_heapq, but won't work for io/_io/_pyio.

You can make the dictionary values as lists for the 'blocked' argument for import_fresh_module(). That would work.

And, can you add documentation for import_module_implementations()? A description how it finds the optimized implementation of a module would be helpful too.

> You can make the dictionary values as lists for the 'blocked'
> argument for import_fresh_module(). That would work [for io].

I don't understand how having multiple modules in the blocked list will help in io case.  io.py will simply not work if _io is blocked.  It does not use "define in Python, override with native if available" strategy.  Instead, io.py and _pyio.py are independent complete implementations.

I don't like that approach because it makes pure python code hard to discover.  In the recent discussions, some people were in fact surprised to learn that pure python io implementation is still available.  The io/_pyio approach also prevents io.py from bring used by alternative python implementations unmodified.


What I can do, is to add an optional "blocked" argument import_module_implementations() along the lines of

def import_module_implementations(name, blocked=None):
    if blocked is None:
        blocked = ('_' + name,)
    ...

This will not solve the io issue, but will add some flexibility.

Note that import_module_implementations() as designed is not very useful for tests of the named module itself.  In that case you need the implementations individually rather than as a list.  This is mostly useful in situations like pickle where other modules are tested for interoperability with alternative pickle implementations.

Of course, I should document the mechanism once we agree on what it should be. :-)  (I did not know that test.support had a reST document, but will update it for this patch.)

> > You can make the dictionary values as lists for the 'blocked'
> > argument for import_fresh_module(). That would work [for io].
> 
> I don't understand how having multiple modules in the blocked list
> will help in io case.  io.py will simply not work if _io is blocked. 

Which you avoid by giving an empty list of blocked modules, using
Alexandre's suggestion.

> I don't like that approach because it makes pure python code hard to
> discover.

Ok, but this code exists and it would be much better if it were
supported.

> The io/_pyio approach also prevents io.py from bring used by
> alternative python implementations unmodified.

It would be foolish to use it unmodified anyway, unless you like
low-speed I/O (and a JIT isn't a magic bullet).

The reason this was done like this is that the io module is imported at
startup: we want to avoid unnecessary parsing of extraneous code (and
unnecessary importing additional dependencies), and we also want to
reduce opportunities for failing to initialize the standard I/O streams
(especially stderr...).

> This will not solve the io issue, but will add some flexibility.

Which is pointless unless such flexibility is needed by someone.

Please don't update the reST document.  The support module should never have been documented outside the module itself, because now we are pretty much committed to otherwise needless backward compatibility for the stuff that is documented.

On Thu, Jul 1, 2010 at 12:18 PM, Antoine Pitrou <report@bugs.python.org> wrote:
..
>> I don't understand how having multiple modules in the blocked list
>> will help in io case.  io.py will simply not work if _io is blocked.
>
> Which you avoid by giving an empty list of blocked modules, using
> Alexandre's suggestion.
>

Yes, you can make import_module_implementations('io') return [io] this way, but what the user is likely to expect would be [io, _pyio].


I understand that there are reasons to keep io the way it is, but most of the other modules should probably follow pickle approach.  In any case, testing alternative io implementations is easy.  No import block trickery is needed - just import io and _pyio separately.

I just don't like the idea of having test.support know details about other modules and would like to have a clear use case before doing anything more sophisticated than blocking '_' + name.

If we end up with a central registry of native optimizations, it should probably not be in test.support.

If not in test.support, then where?  Unless there's some reason for user code or other implementations (or stdlib code itself) to access that map, it seems to me that test.support is exactly where it belongs.

Generic test support (that isn't specific to Python/stdlib testing) should go in unittest (and yes there is stuff currently in test.support that should move to unittest after suitable improvement and discussion...there's an issue about that.)

> there's an issue about that.

That would be #8273.  See also #17037 and PEP 399.

Looks like there is still some disagreement about the implementation here, so this isn't actually ready to commit yet?