[Cantera 2.5] Update valve/etc. in science section

[ischoegl]

Addresses @speth's comment in Cantera/cantera#667

The one remaining to-do for this change is to update the "science" documentation for the reactor model on cantera.org (https://cantera.org/science/reactors.html#reactor-networks-and-devices).

Per @bryanwweber's comment in #88, the update should be parked as a PR until the release of 2.5.

[bryanwweber]

@ischoegl You can rebase this onto master now

[bryanwweber]

@ischoegl What's the status of this PR? Thanks!

[ischoegl]

@bryanwweber ... will have to look over recent changes on the MFC. Will certainly tackle this as soon as beta is reached.

[bryanwweber]

Thanks @ischoegl I agree waiting for beta makes sense.

[ischoegl]

@bryanwweber ... now that 2.5 is on the horizon, I finally updated the docs.

[bryanwweber]

Thank you for updating this! A few formatting comments here. I think it would make more sense to keep the symbols here consistent with the docstrings, keeping K_v instead of c.

One problem is that the :py:meth: role isn't defined, but I can't recall right now how to link to Python documentation methods... I think it's :py:func: with the fully classified name (i.e., MassFlowController.set_time_function). You can shorten the name to just the method by putting a tilde before the class name.

That said, there doesn't actually appear to be any documentation for the various attributes and methods of these classes, see: https://cantera.org/documentation/dev/sphinx/html/cython/zerodim.html#massflowcontroller I'm not sure what happened to those.

[bryanwweber]

@ischoegl Looking at the dev documentation here: https://cantera.org/documentation/dev/sphinx/html/cython/zerodim.html#massflowcontroller I still see K_v as the coefficient symbol. Do the Cython docs also need to be updated? I think people are much more likely to read the Python docs than C++, although I see that the symbols are consistent here with C++.

[ischoegl]

@bryanwweber ... Yes ... I just realized that I had looked at C++ as well (which is why I deleted my earlier declaration to be done). I agree that Python is the more likely avenue, and am mostly done with updating the docs accordingly.

On a related note, what version of Sphinx are we on? Based on the documentation, we may have access to :py:method:, :py:classmethod:, :py:attribute:, etc. (not familiar with how Nikola pulls parts together, so sorry about the question). Nevermind, looked at the wrong part of the docs.

[ischoegl]

OK. I force-pushed - I don't think that :py:var: is the correct syntax for referencing class properties - please let me know and I'll update.

Regarding the differences between C++ and Python, all coefficients are now defined for the parent object using the same private member variable FlowDevice::m_coeff in C++ (which is accessed from the derived class using more specific names, e.g. Valve::setValveCoeff). For Python, those coefficients are set using properties, e.g. Valve.valve_coeff. There isn't an equivalent to a class property in C++, which explains the more generic formulation of the C++ docstrings.

[ischoegl]

@bryanwweber ... finally located the reference I was wondering about: https://www.sphinx-doc.org/en/master/usage/restructuredtext/domains.html#cross-referencing-python-objects (which curiously mentions :py:meth:)

Updated accordingly, so I believe this should be done.

[bryanwweber]

Thanks @ischoegl!

[bryanwweber]

FYI @speth this will require a few more changes to the docs from Cantera/cantera, I'm working on a PR. We should wait to merge this until after 2.5.1.

[ischoegl]

ok. I am aware of remaining activity on the website. The only comment I have is that the new behavior will be default starting in 2.5.0, and I am not sure how long it would take before a minor release of 2.5.1.

Hmm ... I may not have gotten the correct impression based on the comment. If I understand, it is scheduled to be merged, but requires some fixes later on ...

[bryanwweber]

@ischoegl thanks again for updating this. As you can probably see, we've tagged 2.5.1 already which incorporates the fixes to the docs. The problem wasn't with this code, but with the fact that Sphinx doesn't include inherited members by default, so the *.set_... (I forgot the name already 🤣) method from FlowDevice wasn't linking properly to the child classes. It's fixed in Cantera/cantera now, so this is all set.

[ischoegl]

@bryanwweber .... Ok, thanks! I saw your fix earlier today. Just didn’t quite know about the context earlier. All the best for the rollout!