Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

update customise interactions nodebook #117

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

barneydobson
Copy link
Collaborator

@barneydobson barneydobson commented Oct 15, 2024

Summary

  • Update customise interactions to use extensions
  • I have added each extension in its own separate module to make them easier to understand (although mentioning these could be in the same module).
  • These modules I have reproduced in html details. My preferred option would be to use this API like in this, which feels much tidier in terms of formatting and more sustainable as code does not need to be reproduced. But as we discussed in SWMManywhere - it seems that mkdocs-jupyter can't access the API docstrings. I could use a yaml codeblock, which would include the python formatting, but it looks super weird as it is formatted differently from the notebook and can't be stored in a details. Thus, I think this is the best way, but am open to suggesstions.
  • I have done the same way to find this scripts folder as in the another demo with os - though it's a bit ugly. Obviously __file__ would be preferrable, but this doesn't seem to work with mkdocs-jupyter.
  • Bit better default behaviour for river_discharge_order.
  • a few broken hyperlinks
  • I'm going to let this close the issue update documentation for 'customise interactions' and 'river reservoir'. @barneydobson #112 - the river reservoir tutorial needs a lot of work that is not really related to extensions

Fixes #112

Copy link
Collaborator

@liuly12 liuly12 left a comment

Choose a reason for hiding this comment

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

looks good to me

Copy link
Collaborator

@dalonsoa dalonsoa left a comment

Choose a reason for hiding this comment

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

The explanations are good an clear, so all good from that part, but...

I get quite a few deprecation warning when building the documentation, as well as:

ImportError: Missing optional dependency 'tabulate'.  Use pip or conda to install tabulate.

It seems that dependency is missing. Also wrong relative links of the type;

INFO    -  Doc file 'wsimod_models.md' contains an unrecognized relative link './../reference-model/#wsimod.orchestration.model.to_datetime', it was left as is. Did you mean
           'reference-model.md#wsimod.orchestration.model.to_datetime'?


@register_node_patch("my_fwtw", "pull_distributed")
def custom_pull_distributed(self, vqip, *args, **kwargs):
"""A custom handler function."""
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not a handler, I think...


@register_node_patch("my_reservoir", "pull_set_handler", item="FWTW")
def custom_pulls_fwtw(self, vqip, *args, **kwargs):
"""A custom handler function."""
Copy link
Collaborator

Choose a reason for hiding this comment

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

What about a little bit more descriptive explanations? I don't think it matters, but it just looks odd to have lots of functions with exactly the same docstring.

from wsimod.orchestration.model import Model, to_datetime

# Identify the location of the scripts folder
scripts_folder = os.path.join(os.path.abspath(""), "docs", "demo", "scripts")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, this is not super elegant, but I know that jupyter and paths just don't get along very well...

Comment on lines +119 to +131
# <details>
# <summary>Source code</summary>
# <pre><code class="language-python">
# from wsimod.extensions import register_node_patch
#
# @register_node_patch("my_dist", "push_check_handler", item="Node")
# def custom_handler_function(self, vqip, *args, **kwargs):
# """A custom handler function."""
# print("I reached a custom handler")
# return self.push_check_handler\["default"\](vqip)
# </code></pre>
#
# </details>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This does not appear correcly formatted when building the documentation. The tabs are all missing.

image

And if you just make this a code cell instead of embedding it into the markdown cell?


# As we illustrated [above](#illustration), we define our new handlers in a separate module, which we will call `custom_reservoir_handler.py`.
#
# <details>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here: wrong format for the python code:
image

#
# We have the following code in a separate module, which we will call `custom_fwtw_pull.py`.
#
# <details>
Copy link
Collaborator

Choose a reason for hiding this comment

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

And here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

update documentation for 'customise interactions' and 'river reservoir'. @barneydobson
3 participants