Create a test suite for doctests

[kinow]

In theory this runs the doctests in each file of the project.

The issue is that running it locally, it immediately fails with:

ValueError: line 58 of the docstring for cf.data.data.Data.cumsum has inconsistent leading whitespace: '    [[--  1  3  6]'

Fixing that one is easy, just indentation of the docstring. But then it fails again on filearray.py.

  File "/home/kinow/Development/python/workspace/cf-python/cf/read_write/um/filearray.py", line 3, in <module>
    from ..constants import _file_to_fh
ModuleNotFoundError: No module named 'cf.read_write.constants'

There are lot of docstrings (which is really good!), but as they were not being run with the CI, I suspect they got outdated and need fixing.

I think this PR can be left open and commits added fixing the docstring, or it can be closed and re-opened later once docstrings have been fixed. Whichever way is fine for me :-)

Cheers
Bruno

[sadielbartholomew]

Oh, wow! Excellent work @kinow, I didn't think it would be so simple in terms of the actual code lines (not in terms of knowledge on what is required to actually know to put those lines in, I would have had to do a lot of digging first). Thanks 😀

I suspect they got outdated and need fixing.

Yes, I've noticed quite a number of minor docstring issues in my time with the codebase.

I think this PR can be left open and commits added fixing the docstring, or it can be closed and re-opened later once docstrings have been fixed. Whichever way is fine for me :-)

How about I branch off your branch, go through & fix the issues until it passes, then merge your changes here with the fix commits straight after (or bundle them into this PR)? That will save you a lot of tedious work & we can then merge your code straight in from this PR.

[kinow]

Sounds good to me Sadie. Feel free to change the code as much as necessary. And thanks for the review!

[sadielbartholomew]

Hi there @kinow! I'm really sorry this went stale for so long waiting on my action.

The reason is that I forgot to provide you with an update. Not long after my previous comment (April or May 😬) I discussed this with David (cf-python creator) and we had a play around with the draft PR but from that realised that most of our docstrings aren't ready for doc-testing because they tend to begin with pre-created objects i.e. start in a some state that arisen after setup that is not provided in the snippet.

For instance as an arbitrary example, for cf.CellMeasure.mid_range the code example is just:

f.data
f.mid_range()

where the f would need to be set up in order to pass (or indeed fail genuinely) doctest, which can be done easily with our field creation and cf.example_field() methods, but would need some additions to the code. And that is the situation for a large amount, possibly a majority, of our code snippets.

So, in short, there is an awful lot of pre-requisite work that would be needed before this PR could go into cf-python as-is. I created an issue to register that we needed to get the code examples in a state where they set themselves up from scratch (though forgot to reference it to you here). Once that's done this PR can go in but it will be a while before then.

I think this PR can be left open and commits added fixing the docstring, or it can be closed and re-opened later once docstrings have been fixed. Whichever way is fine for me :-)

So it comes back to this. Would you prefer to close this and then I can ping you when the prerequisite Issue is complete such that this can be opened and merged? Or just leave this open until that time? Either is fine for us. Just be aware it may be a while before this can be merged. As a third option, I could merge this as-is (after one review comment I'll raise in a moment) and I'll disable the running of the doctests after. Then we can manage when it gets used, so you don't need to have this PR sitting around for any more time. That would be fine by us too.

As a side note, I am about to add doc-testing to cfunits which is a much smaller library and hence much easier to set up doctesting on. I'm essentially using the code you have outlined here (with much thanks for showing how it is done), so if you'd like I'll happily put you down as co-author for that. Would you prefer that? If so, just let me know your email and I'll set you as co-author for that commit.

[sadielbartholomew]

It is more in line with PEP8 to avoid backslashes for line continuation, so as one minor comment I would suggest changing this as otherwise it might break our test running pycodestyle:

Suggested change

	for importer, name, ispkg in \
	pkgutil.walk_packages(cf.__path__, cf.__name__ + '.'):
	for importer, name, ispkg in (
	pkgutil.walk_packages(cf.__path__, cf.__name__ + '.')):

[kinow]

Hi Sadie!

but from that realised that most of our docstrings aren't ready for doc-testing because they tend to begin with pre-created objects i.e. start in a some state that arisen after setup that is not provided in the snippet.

Makes sense! I never used it, but I think pytest fixtures can be used with doctests too - https://docs.pytest.org/en/stable/doctest.html#using-fixtures (just in case that's helpful).

So it comes back to this. Would you prefer to close this and then I can ping you when the prerequisite Issue is complete such that this can be opened and merged? Or just leave this open until that time? Either is fine for us.

Either is OK for me too.

This PR is a bit outdated, and I think you have a good plan on how to tackle this issue, so I'll close this one.

Feel free to use any parts of the PR if useful, and no worries on authorship/credit :-) For me the most important will be to have doctests that work as documentation+tests, and that are up-to-date to the current code base.

I'm watching the repo, so will keep an eye on the progress 👍

Thanks @sadielbartholomew !

[sadielbartholomew]

Thanks @kinow, that's great. I hope you have a great 2021, while we are conversing briefly here!

[kinow]

Same to you Sadie! Happy 2021! 🥳