JOSS paper review - Documentation

[PeterKraus]

Here I will collate issues/suggestions I found with documentation. See openjournals/joss-reviews#4528 for the full JOSS review.

• Statement of need - the landing page on GitHub has a nice README.md, but the index page of the docs could be expanded with a short blurb. Feel free to reuse something from the paper and don't be afraid of being more technical.
• Functionality documentation - the API docs are built, which is great, but I'm missing a "functionality" page that would link to classes/functions in the API docs. It's up to you if you put it as a subsection of the "Overview" or make a new section. I think such a page should discuss the decisions behind the default output format, link to the key classes/functions in the API docs, and also mention it's possible to customise the output PNGs. Then, reference this overview from the paper to avoid having to write the same thing twice and risking the paper might end up out of date.
• Automated tests - CI is enabled and tests pass. However, it's difficult to figure out what's being tested where. I'd suggest splitting this test file into multiple modules, separating the functionality tests from the validity tests. Also, I think a very useful category of tests is missing - comparing image data directly with a reference png. You can use the instructions here to get a test working. We have written a similar test you can copy, here.

[sgbaird]

@PeterKraus I've updated the index page with a blurb, added references to the API in a Limitations and Design Considerations section (as well as in the basic usage section), and split xtal2png_test.py into 4 modules each with a one-line docstring at the top:

I like your suggestion about comparing reference images as a category of testing and this is something I think I've considered before. I've added a test that saves the images to files, and then uses the loaded versions of those saved images for decoding.

As far as having fixed reference images to compare against, I'd like to hold off on this one until the default API becomes stable (i.e. v1.0.0), which I'm planning to do in conjunction with posting a preprint for a results manuscript. I'm planning to update many of the defaults based on results that I've yet to obtain and possibly along the way. I'll keep an issue open as a reminder of this. Does this seem reasonable?

Let me know if I missed something or if something seems off about the changes. Thanks for the feedback!

[sgbaird]

I also added an examples section:

Let me know what you think!

[PeterKraus]

That's great, thanks. Especially the examples are very useful.

As for the testing, it might be useful to test more than one platform. Also, you can in principle generate a reference image now and always replace it once you change the defaults, but that's your call.

Feel free to close the issue!