documentation missing usage / how to launch

[brandondube]

This issue is part of my portion of the JOSS review, openjournals/joss-reviews#5478

The documentation contains three tutorials, which use the GUI. There is not anywhere in the documentation (as best I can find) that shows how to launch the GUI, or even how to import PyPO when using the tool for scripting

[brandondube]

More generally, the documentation is extremely minimal. Please create at least one tutorial showing how to use the software for scripting (a feature that is described in the paper). Some documentation of the API (in the ideal limit, a la Scipy/Numpy) should also be present, beyond auto-generated stubs showing the constructors and fields of structs, classes, etc.

A set of community guidelines and/or CONTRIBUTING.md are needed to complete the JOSS submission, as well.

[arendMoerman]

The documentations have been updated to include more tutorials.
Specifically, we added an extra set that showcases the fundamentals of using PyPO, such as input dictionary types for public API methods, gridding options for reflectors, etc.
Also, the first GUI tutorial now contains a first section on how to launch the GUI.

There was another set of tutorials in the form of interactive jupyter-notebooks. These were a bit more on using PyPO in scripts and the functionalities.
These tutorials were already present when we first submitted PyPO, but in a different repository on the organisation page, so maybe hard/impossible to find without further instructions, which were indeed lacking in the original docs.
We have now put them in the main repository, under the tutorials/ directory. Also, the docs now include these tutorials as statically rendered HTML.

We have now made a separate API reference, containing all public methods for interacting with PyPO.
In addition, we have added a full software documentation that documents the entire backend of PyPO.

A separate page has been made describing how contributions should be carried out using the fork-and-branch workflow. This page also contains guidelines on how to comment code so that documentation can be generated from it using doxygen, for example.

Is the current state of the docs acceptable?

[brandondube]

Thank you for greatly enhancing the documentation! A few residual comments:

On https://pypo-dev.github.io/PyPO/basictut1.html

• Typo: PyPO uses the [CMake]() build system to compile (missing/incomplete/broken link)
• which is part of the standard packages on Python (maybe mention PEP number?). I don't think the parenthetical (note-to-self) should be in the released version of the documentation
• The section that discusses memory management seems somewhat inconsistent; it mentions that Python handles all (de-)allocation, but at the end says the bottom layer deletes all internally allocated memory structures., so both layers do their own memory management?
• Typo: visualisation if optical systems and results,
• I am not sure this page is really a tutorial? The user is not shown any code, is not asked to write any code, and is not shown any steps/processes/etc that would normally be a tutorial. I think this page would be more of an "Explanation" in The Documentation System

On https://pypo-dev.github.io/PyPO/basictut2.html

• I am not sure this is a tutorial either? It shows how to import the object and how to use one method, related to logging. But it doesn't show me how to actually do anything with the system

On https://pypo-dev.github.io/PyPO/basictut3.html

• Missing link: A Numpy array of length 2, this parameter sets the number of cells along the gridding axes. Read more here. (last 3 words)
• A special case arises when 'a' = 'b'. In this case, the paraboloid is a so-called [surface of revolution](https://en.wikipedia.org/wiki/Surface_of_revolution), and is symmetric around the central symmetry axis. In this case, the paraboloid has two focii: one at a finite distance from the vertex (the bottom of the 'bowl' of the paraboloid) along the axis of symmetry and one at infinity. this sentence is not really correct/in keeping with the norms of optics. A focus is a definite point, a focus at infinity does not really make any sense.
• Hyperboloid Geometries -- elsewhere in the docs, 'quadric' is used to describe the type of surface the software is able to model. A hyperbola is not a quadratic, but spheres, parabolas, hyperbolas, and ellipses are all conics. I think the documentation (and paper) should say conic where they currently say quadric.
• This page also feels more like an API reference, and not a tutorial. I am not able to do anything with these types of surfaces by reading this page; actually, I do not even know how to (in code) describe one of these surfaces with the software! There is no line of code showing how to use anything, just the meaning of the parameters (an API reference)

The refrain that these are not really tutorials applies to the "gridding" and "some more" pages, as well

I will review the jupyter notebook based content later this weekend

[arendMoerman]

Thanks a lot for the detailed comments, especially the link to the documentation system! Wish I knew about that earlier...

I have moved all these "basic tutorials" to a new explanations page, as they are indeed more like explanations than tutorials. Also, I agree with and have implemented almost all other residual comments.

The only point I do not agree with is the renaming of the surfaces from quadric to conic. A conic, in my understanding, is the curve traced out by intersecting a plane with a cone. The curve itself is described as the locus of a quadratic equation in two variables (a quadratic curve), and hence the set of points describing the curve is inherently 1D, in the sense that you can continuously map the conic onto the line of real numbers.

A quadric surface, on the other hand, is defined as the locus of a quadratic equation in three variables.
Since the hyperboloid equation consists of terms of at most degree 2, I think it is still a quadric surface.

Please see this link. On page 13 (69 on the printed document), the general equation for a quadric surface is given, which I believe also encapsulates the equation for a hyperboloid of two sheets (the upper sheet is used in PyPO). Also, on page 21 (77 in print), they discuss the hyperboloid as a quadric surface, so I do believe that "quadric surface" is an appropriate term here.

[brandondube]

Thank you; I looked over the written documentation, and it looks good. I was not able to check that the GUI works as described in the docs. The PySide2 dependency is wheel-only, and does not support darwin-arm64:

https://pypi.org/project/PySide2/#files

With regards to quadratic; my apologies, we come from different backgrounds. I assumed quadric was a synonym or alternative spelling for "quadratic." It would appear it is really a specific class of geometry, which all of the contemporary conics of optics are members of. You may find users from an optics background are confused by the term as I have been, but it is technically correct as-is

[arendMoerman]

Thank you for the observation on the arm64. We have changed the dependency on PySide2 to PySide6 - according to its PyPi page, it has a universal2 wheel for macosx. From what I could find, this also includes arm64 support.

Could you please confirm if PySide6 installs correctly, and with that, if the GUI can be started?

And I agree that the term "quadric" could be confusing for people used to certain conventions from optics, even though it is a mathematically correct term. We have now included a few lines in the explanation on "Reflectors/elements in PyPO" where we state the common usage of "conics" in optics and why we choose to use the term "quadric". I hope that this clears up some potential confusion.

[brandondube]

Thanks, all of that works well. One mild nit -- on MacOS, Qt conforms to the OS norms, and the menu bar is used. In, e.g. this tutorial, you say

From the elements menu select Add Reflector > Quadric surface.

My first thought on reading this was that the element menu was the set of four tabs beneath the logo (on macos), because the menubar is at the top of the screen / outside the actual application window. You might consider adding a bit of clarification there

Otherwise, everything LGTM