Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixes for doc building; minor docstring updates #467

Conversation

stevebachmeier
Copy link
Contributor

Fixes for doc builds

Description

Changes and notes

We previously worked on the content of the results system docstrings

without paying too much attention to whether or not we could actually get
doc builds to pass (let alone how they look). This fixes the failing
docbuilds and makes other minor tweaks to docstrings.

Testing

tests pass and docs build.


.. todo::

Everything here.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Next PR

@@ -132,9 +130,11 @@ def add_stratification(
Raises
------
ValueError
- If the stratification `name` is already used.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn't rendering well.


This module provides a :class:`ResultsInterface <ResultsInterface>` class with
methods to register stratifications and results producers (referred to as "observations")
to a simulation.
"""

from __future__ import annotations
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already had determined that we couldn't use, e.g. pd.Series[str] (which requires this import to work currently), but this import itself was throwing warnings about not being able to resolve pd.Series or pd.DataFrame - it was super difficult to track this down as the culprit!

@@ -21,6 +22,8 @@ py:class Time
py:class vivarium.framework.time.Time
py:class Timedelta
py:class vivarium.framework.time.Timedelta
py:exc ResultsConfigurationError
py:exc vivarium.framework.results.exceptions.ResultsConfigurationError
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's happening here? Shouldn't references within vivarium itself just work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/mnt/share/homes/sbachmei/repos/vivarium/src/vivarium/framework/results/context.py:docstring of vivarium.framework.results.context.ResultsContext.set_default_stratifications:1: WARNING: py:exc reference target not found: ResultsConfigurationError

Not sure what the deal is. I think I was able to get it to build if I used the full path to the ResultsConfigurationError object in the Raises docstring, but that's just ugly

def set_default_stratifications(self, default_grouping_columns: List[str]) -> None:
    """Set the ...

    Raises
    -------
   vivarium.framework.results.exceptions.ResultsConfigurationError
        If the `self.default_stratifications` ...
    """
    ...

The provided :class:`BaseObservation` class is an abstract base class that should
be subclassed by concrete observations. While there are no required abstract methods
to define when subclassing, the class does provide common attributes as well
as an `observe` method that determines whether to observe results for a given event.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this say a to_observe method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite. There is a to_observe attribute (which is a Callable that returns a boolean), but I'm highlighting the observe method that is really the thing that calls a given observation's results_gatherer method (if to_observe() returns true for a given event)

@stevebachmeier stevebachmeier merged commit 427f5f6 into epic/results-documentation Aug 19, 2024
6 checks passed
@stevebachmeier stevebachmeier deleted the feature/sbachmei/MIC-5097-5220-fix-doc-builds branch August 19, 2024 20:29
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants