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

Docs enhancements #235

Open
wants to merge 33 commits into
base: master
Choose a base branch
from
Open

Docs enhancements #235

wants to merge 33 commits into from

Conversation

aerispaha
Copy link
Member

@aerispaha aerispaha commented Dec 18, 2024

Major overhaul of the swmmio documentation that better organizes the API reference documentation, adds a User Guide with examples (including some silly examples), and provides a more concise introduction. I've also fixed most of the docstrings that were incorrectly formatted (although not all of the docstrings have been standardized).

Draft Docs

You can review the unreleased docs here

swmmio.Model from URL

Also included is a small new feature that allows us to instantiate a swmmio.Model object with a url to a INP file somewhere on the network. This was handy for quickly testing things and made some of the docs more concise. For example, we can work with a big model hosted in the NCIMM-Black-White-Box repo like this

# url to the raw inp file 
URL = 'https://raw.githubusercontent.com/SWMMEnablement/NCIMM-Black-White-Box/25a7dc8fc58f67d15954679f294d09b9061766a4/SWMM5_NCIMM/10070_H_Elements.inp'

# instantiate a model object by passing in the URL 
model = swmmio.Model(URL)

This will download the remote INP file to a temp location, then instantiate the Model object as usual.

Closes #233
Closes #171
Closes #236
Closes #234

@aerispaha aerispaha marked this pull request as ready for review December 18, 2024 19:57
@bemcdonnell bemcdonnell requested a review from karosc December 18, 2024 20:22
@bemcdonnell
Copy link
Member

Adding @karosc for some responses too. @aerispaha can you drop the link in for the draft built docs?

@bemcdonnell
Copy link
Member

Also going to add @wraseman here for some thoughts too. He's a fresh SWMMIO user and it would be good to get his reaction.

@aerispaha
Copy link
Member Author

aerispaha commented Dec 18, 2024

@bemcdonnell and team, you can see the draft built documentation on readthedocs, here).

I see that there is a weird thing going on in the Making art with SWMM guide. I think it's because I have some caching going on. I'll fix that. Fixed.

@bemcdonnell
Copy link
Member

@aerispaha, for starters, you've done such a nice job on the documentation overhaul! I like the focus and the new feel - lighter and cleaner. The only suggestions I have are for the User Guide. I think it would be useful to make a sub header section (BOLD) how to "Edit model Parameters." Get the attention of those who are quickly scrolling through that you can use this to Edit/modify models. Also, for running models, you might cite PySWMM as the vehicle that is used. Lastly, maybe consider a separate page for notebooks and add it to the TOC.

We can also provide documentation links from www.pyswmm.org if you'd like!

Keep up the great work on this! I think it is an asset to the community and so are YOU!

@wraseman
Copy link

Working through the docs, providing feedback on README.md and the first two sections of Getting Started.

README.md

  • Image: it isn't clear to me what the figure is supposed to be: docs/_static/img/swmm-zoom-graphic.png. Is it a logo? Is it a swmmio output? Would recommend one of the images in the docs (e.g., Visualizing SWMM Models — swmmio 0.7.4.dev0 documentation)
  • Version number: there are three different version numbers README.md, swmmio.readthedocs.io, and releases. Which is correct?

Getting Started

  • Dependencies: need to add requests or from swmmio.tests.data import fails

Getting Started: Edit Model Parameters

  • The last DataFrame is intended to show how the user can modify the model and write it to a new .inp file. However, none of the modified values (i.e., OutfallType, StageOrTimeseries) are visible in the DataFrame shown in the tutorial.

Getting Started: Building Variations of Model

  • For climate change scenario, I would have expected the stage to increase from 576 to 581 (5ft rise). But instead, it increases to 628.5 ft.
  • In the same example, the 'baseline' variable gets overwritten in the loop. Since it is the baseline, shouldn't it remain unchanged? I'd recommend setting another variable, like 'slr'.

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