X11 test in test_rmagic.

Issue #181 resolved
Dav Clark created an issue

This test was added in commit 30c7477, and it seems to work for the core purpose (it fails), but then other tests in test_rmagic break.

I'm leaving the X11 code in the ipython magic broken for now until we can figure this out (the X11 code fix is pretty trivial, and should likely be moved partially into python so we are less likely to have such problems in the future!).

Comments (14)

  1. Dav Clark reporter

    This is likely relevant to the work @takluyver is doing on display for R in the IPython notebook.

  2. Thomas Kluyver

    I'd guess you need the final line to set it back to png in a finally: block - otherwise when the test fails, it won't run the last line.

    For now, I'm not offering different backends in the native R kernel, I'm forcing it to go through png. I'm sure that will change in time, though.

  3. Dav Clark reporter

    So, just to clarify / apologize. I found the problem that you fixed in 5c520f810cad, and by way of trying to "be a good contributor." I endeavored to write a test before fixing the actual problem with the R-code-in-a-python-string. Then, it turned out the testing framework was using nose-specific features (the yeild construct in a test_* function), and so those plot tests weren't running at all...

    I fixed that, and realized that the test now breaks the whole testing framework if, e.g., X11 doesn't work, because it leaves the system configured to use whatever graphics subsystem was operative before a failure. I.e., it tends to leave a broken graphics subsystem configured.

    At this point, I figured it was time to take a step back, and then got busy. I should've left the testing framework in a less precarious state (as you've now done). And moreover, I should've just fixed the bug that I found (but at least you saw it and did that).

    In any case, it seems that:

    1. we should be clear on style. In particular, I think R code embedded in strings is very much not the way you (@lgautier) do things, and I should convert that python string to actual rpy2 python wrapper code.

    2. The graphics tests should be fixed to return the graphics output to png at the end. However, we should probably also set up a condition so that if R doesn't have X11 graphics, that test shouldn't run. I'm not sure how to do this off the top of my head, but I guess it shouldn't be hard. If you know OFF THE TOP OF YOUR HEAD let me know :).

    3. Should we be using a slightly more sophisticated testing framework? There are two approaches. We can continue to only use features that don't require the framework, or we could go ahead and use nose or pytest specific features. In either case, it'd be good to agree on which one. My vote currently would be for pytest (though nose2 looks like a nice, lightweight approach).

  4. Laurent Gautier

    No problem. About your points. 1. style: I have my preferences, sometimes. However, personally I do not feel the urge to convert the strings... as long as they are working as intended. One note though: the missing part of the R expression, the result of a copy/paste accident I am guessing, would have been caught earlier if not in a string. 2. Just close all opened graphical devices in R.

    # Top-of-head: completely untested
    grdevices = importr('grDevices')
    for dev_i  in grdevices.dev_list():

    Then open a PNG device. 3. I have been sticking to Python's default unittest to minimize the number of dependencies. Historically rpy2 has been quite progressive with respect to Python 3 - it had it working before numpy did for exampe - and I was seeing any dependency as a possible problem. The situation of Python 3 seems much better nowadays; may be I should revisit this.

  5. Thomas Kluyver

    Re test frameworks: pytest and nose both have robust Python 3 support now.

    In favour of pytest: Nicer (I think) default output, conveniently allows you to write checks like assert a == b and still get helpful errors, whereas nose expects you to use nt.assert_equal(a,b).

    In favour of nose: Probably a bit more widespread, and it's specified as part of the 'Scipy stack' for people making scientific Python distributions. Some people feel uneasy with the 'magic' pytest does.

    I haven't used nose2 yet.

  6. Laurent Gautier

    Your comment about pytest making some people uncomfortable with the amount behind-the-scene happening would keep me away.

    nose2 is Python>=2.7, but that's the same for rpy2 so no issue there. nose1 is in maintenance mode, and development around nose2 is slow (according to its author).

    This leads me to the question: what the features in node that are not in unittest2 (since unittest == unittest2 since Python 2.7, IIUIC) ?

  7. Dav Clark reporter

    Next time I'm doing some rpy2 development, I'll start looking into using a test-runner, but I won't make any changes that disable functionality with plain unittest2 for now. I'll report back if I have anything useful to say - probably I'll go the pytest route. It's stable and easier (for me) to read the output.

  8. Dav Clark reporter

    X11 test currently disabled, but X11 functionality working. Testing working X11 graphics seems super-low priority.

  9. Laurent Gautier

    @davclark . Do you think that we can reach a state with reasonable testing with unittest2 for rpy2-2.4.0 ? I am fine with experimenting with testing frameworks for rpy2-2.5.0, but I am reluctant for rpy2-2.4.0 (I'd like to release soon - I should be done fixing something around R language objects at the lower level, and then should be ready to go).

  10. Laurent Gautier

    I have (finally) "healed" the branch "default". It drifted way too much, so I just reset it to the current version_2.4.x and edited the version numbers. I will cherry pick changes that were in the previous "default" as needed.

    If you want to change the way testing it done, default is where you will want to start.

  11. Log in to comment