metoppv / improver

IMPROVER is a library of algorithms for meteorological post-processing.
http://improver.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
103 stars 85 forks source link

Proposal: test module documentation title convention removal #2004

Closed cpelley closed 2 months ago

cpelley commented 4 months ago

Raised in https://github.com/metoppv/improver/pull/2000#discussion_r1625656917. This issue is to capture discussion around the removal of test module description of the file it tests. An example:

https://github.com/metoppv/improver/blob/dee4f9b1ac20b6c3adfab0d124fdc5be54b6e344/improver_tests/utilities/test_cube_checker.py#L5

Why remove?

Is the test module description of the file it tests not superfluous? Each function or class has an associated test file which is indicated by its name and directory path. I just wonder whether such conventions are necessary for the tests (the more conventions, the more time spent discussing and reviewing them). I think anything we can do to reduce the burden on the review the better (hence the benefits of tools like black, flake8 etc.).

What this proposal is not

This does not intend to imply a convention for prohibiting test module documentation. As with all modules, if there is merit to module documentation, then by all means write it.

Issues

bayliffe commented 4 months ago

Sounds like a reasonable idea to me.

Katie-Howard commented 2 months ago

I've had a think about the proposal and I think @cpelley has a good point. If it's just saying it's a test (which is obvious because it literally says it in the name and is the unit testing part of the code) then we probably don't need to then write it again. If it's something that isn't obvious it might be necessary but otherwise I'm happy 👍

cpelley commented 2 months ago

Thanks @Katie-Howard, ping @gavinevans I think we have enough agreement to consider this 'decided'. I have checked the developer guidelines concerning testing in https://github.com/metoppv/improver/blob/master/doc/source/Code-Style-Guide.rst#unit-testing and I don't think any update is required as a result of this decision. Thanks all.