Improved autoenum documentation by jsiirola · Pull Request #3389 · Pyomo/pyomo

jsiirola · 2024-10-22T18:05:15Z

Fixes # .

Summary/Motivation:

NOTE: This PR is built on #3382. Please review / merge that PR before looking at this one. If you want to get a head start, you can compare this branch against the doc-reorg branch.

@shermanjasonaf pointed out that #3382 was not rendering Enums correctly. Upon further investigation, enum-tools.autoenum does not play nicely with sphinx.ext.autosummary (which we use to auto-generate the Library Reference). In addition, enum-tools does not pick up Sphinx-style Enum member documentation unless we decorate the enums (adding a runtime dependence on enum-tools).

This PR drops our use of enum-tools and replaces it with our own implementation of autoenum. Now that the Enums are being documented, the PR needed to update them to silence numerous Sphinx warnings.

Changes proposed in this PR:

Drop use of enum-tools
Implement a custom Sphinx extension for automatically documenting enums
Update Enums to silence Sphinx warnings

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

I agree my contributions are submitted under the BSD license.
I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

codecov · 2024-11-01T03:13:32Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.64%. Comparing base (69f1937) to head (ba5ef0f).
Report is 18 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3389      +/-   ##
==========================================
+ Coverage   88.58%   88.64%   +0.06%     
==========================================
  Files         883      883              
  Lines      100289   100916     +627     
==========================================
+ Hits        88841    89461     +620     
- Misses      11448    11455       +7

Flag	Coverage Δ
linux	`86.04% <100.00%> (-0.01%)`	⬇️
osx	`76.03% <100.00%> (+<0.01%)`	⬆️
other	`86.53% <100.00%> (-0.01%)`	⬇️
win	`84.38% <100.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

doc/OnlineDocs/ext/pyomo_autosummary_autoenum.py

blnicho · 2024-11-04T17:57:03Z

pyomo/common/enums.py

+            """A compatibility wrapper around :class:`enum.IntEnum`
+
+        This wrapper class updates the :meth:`to_bytes` and
+        :meth:`from_bytes` docstrings in Python <= 3.12 to suppress


Will this need to be updated to Python 3.13 after #3401 is merged or is this fixed in 3.13?

According to the Python issues, this is supposed to be fixed in Python 3.13

blnicho · 2024-11-04T18:01:52Z

pyomo/contrib/solver/results.py

+    "The solver exited because convergence criteria of the problem were satisfied."

    maxTimeLimit = 1
+    """The solver exited due to reaching a specified time limit."""

    iterationLimit = 2
+    """The solver exited due to reaching a specified iteration limit."""

    objectiveLimit = 3
+    """The solver exited due to reaching an objective limit. For example, in
+    Gurobi, the exit message "Optimal objective for model was proven to
+    be worse than the value specified in the Cutoff parameter" would map
+    to objectiveLimit.
+    """

    minStepLength = 4
+    """The solver exited due to a minimum step length.  Minimum step length
+    reached may mean that the problem is infeasible or that the problem
+    is feasible but the solver could not converge.
+    """


Are there negative implications to moving these descriptions out of the class docstring? For example, if a user calls help on one of these enum classes will they see all the available options?

Yeah - there are. You are right: help() will not pick up the Enum member docs when they are provided inline like that. That said, documenting the enum members as attributes in the class docstring made for problems in Sphinx (at a minimum, there were warnings -- but I think it was actually leading to cross-referencing errors). I made the call that it was a higher priority to have good online docs (even if it caused problems for help()). I am happy to be overridden, though.

I'd prefer that the help messages stay for this one purely because it is new / we want visibility into what the new solver interface is doing.

blnicho · 2024-11-04T18:03:29Z

pyomo/core/base/suffix.py



-class SuffixDirection(enum.IntEnum):
+class SuffixDirection(IntEnum):


You didn't move the descriptions of the enum options out of the docstring like you did for others. Is this intentional?

I was mostly focused on silencing warnings from Sphinx (I would like to turn on the "map warnings to errors" in the Sphinx configuration to protect us from our future selves). These weren't tossing warnings because they are not actually documenting the Enum members. In the long run, we should definitely move the documentation so that it will be picked up -- but in the spirit of managing the complexity / size of PRs I was going to punt that to later.

mrmundt · 2024-11-04T20:05:24Z

pyomo/contrib/solver/results.py

-    Attributes
-    ----------
-    noSolution: 0
-        No (single) solution was found; possible that a population of solutions
-        was returned.
-    infeasible: 10
-        Solution point does not satisfy some domains and/or constraints.
-    feasible: 20
-        A solution for which all of the constraints in the model are satisfied.
-    optimal: 30
-        A feasible solution where the objective function reaches its specified
-        sense (e.g., maximum, minimum)


Ditto here - can these be put back so the documentation is easier for folks?

I would really rather not move the documentation into the class docstring for several reasons:

it requires us to duplicate information (the enum value is manually written into the class docstring, and there is no obvious way to write a test to ensure that the documentation stays up-to-date).

it leads to the following Sphinx warnings:

[pyomo]/pyomo/contrib/solver/results.py:docstring of pyomo.contrib.solver.results.SolutionStatus.noSolution:1: WARNING: duplicate object description of pyomo.contrib.solver.results.SolutionStatus.noSolution, other instance in api/pyomo.contrib.solver.results.SolutionStatus, use :no-index: for one of them [pyomo]/pyomo/contrib/solver/results.py:docstring of pyomo.contrib.solver.results.SolutionStatus.infeasible:1: WARNING: duplicate object description of pyomo.contrib.solver.results.SolutionStatus.infeasible, other instance in api/pyomo.contrib.solver.results.SolutionStatus, use :no-index: for one of them [pyomo]/pyomo/contrib/solver/results.py:docstring of pyomo.contrib.solver.results.SolutionStatus.feasible:1: WARNING: duplicate object description of pyomo.contrib.solver.results.SolutionStatus.feasible, other instance in api/pyomo.contrib.solver.results.SolutionStatus, use :no-index: for one of them [pyomo]/pyomo/contrib/solver/results.py:docstring of pyomo.contrib.solver.results.SolutionStatus.optimal:1: WARNING: duplicate object description of pyomo.contrib.solver.results.SolutionStatus.optimal, other instance in api/pyomo.contrib.solver.results.SolutionStatus, use :no-index: for one of them

The online docs don't pick up that those are actually documenting the Enum members. You get this:

instead of

While you help() won't show the attribute descriptions (because Python doesn't allow attributes to have __doc__ members), I would personally prefer that given decision that forces us to do a trade between the (searchable, indexed, and browsable) online docs and the interactive help(), I would lean into the online docs. For the record, the help() looks like this if the attributes are documented in the class docstring:

>>> from pyomo.contrib.solver.results import SolutionStatus >>> help(SolutionStatus) class SolutionStatus(enum.Enum) | SolutionStatus(value, names=None, *, module=None, qualname=None, type=None, start=1, boundary=None) | | An enumeration for interpreting the result of a termination. This | describes the designated status by the solver to be loaded back into | the model. | | Attributes | ---------- | noSolution: 0 | No (single) solution was found; possible that a population of solutions | was returned. | infeasible: 10 | Solution point does not satisfy some domains and/or constraints. | feasible: 20 | A solution for which all of the constraints in the model are satisfied. | optimal: 30 | A feasible solution where the objective function reaches its specified | sense (e.g., maximum, minimum) | | Method resolution order: | SolutionStatus | enum.Enum | builtins.object | | Data and other attributes defined here: | | feasible = <SolutionStatus.feasible: 20> | | infeasible = <SolutionStatus.infeasible: 10> | | noSolution = <SolutionStatus.noSolution: 0> | | optimal = <SolutionStatus.optimal: 30> |

and when only the attributes are documented (using PEP257), you get

>>> from pyomo.contrib.solver.results import SolutionStatus >>> help(SolutionStatus) class SolutionStatus(enum.Enum) | SolutionStatus(value, names=None, *, module=None, qualname=None, type=None, start=1, boundary=None) | | An enumeration for interpreting the result of a termination. This | describes the designated status by the solver to be loaded back into | the model. | | Method resolution order: | SolutionStatus | enum.Enum | builtins.object | | Data and other attributes defined here: | | feasible = <SolutionStatus.feasible: 20> | | infeasible = <SolutionStatus.infeasible: 10> | | noSolution = <SolutionStatus.noSolution: 0> | | optimal = <SolutionStatus.optimal: 30> |

Okay, I am comfortable with this change then

jsiirola and others added 16 commits October 22, 2024 11:45

Replace enum-tools.autosummary with our own plugin

58722be

Use a custom template for documenting Enums

690ae7a

Move the remaining IntEnums in Pyomo to use pyomo.common.enums.IntEnum

940de78

Improve documentation for IntEnum wrapper class

8e16275

Silence documentation warnings (due to duplicate docs)

79f14b1

NFC: fix typos

aadc361

Merge branch 'doc-reorg' into doc-autoenum

0564f6f

Updates for Python 3.8 compatibility

7da5083

Update Sphinx version

9fe85a7

Update python version for ReadTheDocs builds

a022dfc

Update comment on minimum Sphinx version

85c21b1

Merge branch 'main' into doc-autoenum

ae1f1f0

Resolve incompatibility with Sphinx<7.2

5c18661

Minor improvements to common.enums docs

e9b92a5

NFC: fix comment typos

90b569a

Relaxing Sphinx version to previous requirement

ba5ef0f

blnicho reviewed Nov 4, 2024

View reviewed changes

jsiirola added 2 commits November 4, 2024 11:18

Merge branch 'main' into doc-autoenum

4baf960

NFC: resolve typos

f8f7f0f

blnicho approved these changes Nov 4, 2024

View reviewed changes

mrmundt reviewed Nov 4, 2024

View reviewed changes

Merge branch 'main' into doc-autoenum

3e67375

mrmundt approved these changes Nov 5, 2024

View reviewed changes

blnicho merged commit 4242394 into Pyomo:main Nov 5, 2024

jsiirola deleted the doc-autoenum branch November 6, 2024 21:24



		class SuffixDirection(enum.IntEnum):
		class SuffixDirection(IntEnum):

Conversation

jsiirola commented Oct 22, 2024 • edited by blnicho Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fixes # .

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

Uh oh!

codecov bot commented Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jsiirola commented Oct 22, 2024 •

edited by blnicho

Loading

codecov bot commented Nov 1, 2024 •

edited

Loading