Hello @soupault! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file doc/examples/applications/plot_haar_extraction_selection_classification.py:

Line 137:80: E501 line too long (80 > 79 characters)

Comment last updated at 2019-05-16 21:16:51 UTC

@stefanv @Borda @hmaarrfk I'm still working on the solution for the 2 aforementioned issues. The reviews are most welcome, but the PR is not yet finalized :).

@soupault Sorry for jumping on you prematurely ;)

OK! Let me summarize the PR once again:

1. Additions:
1.1 test the function/class docstring examples
1.2 build the documentation
1.3 test the gallery examples

2. Changes:
2.1 Configuration file syntax has been changed to bash to improve its clarity.
2.2 doc/source/random_gallery.py used platform-dependent path delimiter -> fixed.
2.3 applications/plot_haar_extraction_selection_classification.py example required wrapping the multiprocessing code using "if-name-main" construction on Windows, otherwise the testing falls into the infinite loop. The issue is described here in details here - https://stackoverflow.com/questions/20222534/python-multiprocessing-on-windows-if-name-main. In short, Windows doesn't have fork, so for multiprocessing code it starts extra Python processes. Those processes import the file, but as long as it contains the multiprocessing code at the top level, we are getting an infinite spawn tree. If interested, see this build for the console log - https://dev.azure.com/scikit-image/scikit-image/_build/results?buildId=322. To overcome the issue and maintain the simplicity of the example I decided to use synchronous Dask scheduler on Windows (i.e. no multiprocessing). I also clarified the language in several places.
2.4 Several docstring examples (1.1) started to fail on Windows (see https://dev.azure.com/scikit-image/scikit-image/_build/results?buildId=344). Initially, I thought that the conftest.py file is not being discovered on Windows. However, after thorough debugging (see pytest-dev/pytest#5194) we found that the conftest.py is actually discovered and properly loaded. I now think that the issue is related to the way numpy prints the arrays of platform-dependent dtypes (IMPORTANT: doesn't matter whether the OS is 32 or 64bit!) when np.printoptions are set to legacy mode. Eventually, I've found that the printing is consistent when the troublesome integer ndarrays are casted to np.int. Furthermore, I've noticed that some of the docstring examples already used this conversion here and there.

@scikit-image/core The PR is ready now :).

i can see the value of running the examples on windows, maybe, but I think the modifications to the docstrings are extremely distracting.

I think we test the library enough to be able to forego docstring testing on windows, in favour of teaching coding style by example.

@soupault Thank you for the heroic slogging! 🦸 Overall, I agree with @hmaarrfk's line of thinking: the examples are there to help our users learn the library. We, as developers, should build whatever scaffolding is necessary to protect that ideal.

Yeah. Sorry for making it sound negative. I hope me taking time to give my opinion speaks louder than my "-1" on certain changes.

These kinds of maintenance PRs are what allows the library to move forward with confidence. Great work hunting down a potential bug with numpy printing.

@hmaarrfk @stefanv No worries and thank you for the comments!

the examples are there to help our users learn the library

I think we are all aligned on that :). I was just trying to get as coverage as possible. You are probably right that if some of the docstring examples starts to fail on Windows, and not on the other platforms, we might need to review our test suite.

It seems that the actual issue(?) is on numpy side (numpy/numpy#13468), so I have disabled --doctest-modules for now.

Otherwise, the PR is as fresh, shiny, and green as before.

Making the executable docstrings fully portable is indeed tricky--for NumPy and SciPy I think we do two things for this:

1. Typically just run the docstring checks on a single platform in CI
2. Improve portability by using refguide_check.py, which falls back to return np.allclose(want, got, atol=self.atol, rtol=self.rtol) when vanilla doctest doesn't think the results match; this almost certainly solves the problem you describe in the cognate NumPy issue

When I check the different results you mention in the cross-linked NumPy issue, I can see the refguide will likely indicate that these are the same because np.allclose(np.array([2, 0], dtype=np.int32), np.array([2, 0], dtype=np.int64)) is True, even though their reprs differ.

Vendoring refguide_check.py would make a bit more work though. There's also pytest-doctestplus, proposals to start fusing it with refguide features, and another solution in sympy, I think.

I've also been dreaming about doctest line coverage reports, described elegantly by Raymond Hettinger.

@tylerjereddy refguide_check looks like exactly the right medicine! Thanks for the pointer!

As far as I know we are already measuring doctest coverage, though we don't distinguish between doctest and unit test coverage...

@jni can you link me to an example coverage html for a module where the docstring examples are shown as "hit" / highlighted for scikit-image?

When did we remove coverage upload, I thought I only silenced it, the same way numpy silenced it.

Thank you @tylerjereddy for your comment!

@jni Even if that refguide_check.py thing presumably solves our issue, I think, integrating it would make our CI way more complicated. Please, let me know if you are ok with the current state of the PR.

Travis fails due to #3891 change and is not related to this PR.

@soupault I hope I answered your question appropriately. =D

Thanks Everyone for the reviews!