Getting started tutorial - #293
Conversation
Co-authored-by: DeepSeek-V4 Pro
Co-authored-by: DeepSeek-V4 Pro
Co-authored-by: DeepSeek-V4 Pro
Co-authored-by: DeepSeek-V4 Pro
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #293 +/- ##
=======================================
Coverage 94.39% 94.39%
=======================================
Files 47 47
Lines 9686 9686
=======================================
Hits 9143 9143
Misses 543 543 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
camUrban
left a comment
There was a problem hiding this comment.
Thanks so much for your work @TianHengZhuang! This looks like a great start. Could you commit changes with the following fixes, and then re-request a review? Before you re-request a review, you should also mark each comment as resolved.
Also, as you are a new contributor, be sure to add your name to the bottom of the list of contributors in the README.md file. Us one of the pre-existing patterns for formatting.
Lastly, would you mind revising your PR's description to follow .github/pull_request_template.md? That really helps keep everything organized.
|
One more thing I forgot to mention: it looks like the lint CI action and two of the pre-commit-hooks actions are failing. This likely means you don't have pre-commit installed yet, as that would have prevented you from committing. The easiest way to get it set up is to follow the steps in |
Address inline review comments, add contributor to README, and fix CI/lint issues. Co-authored-by: DeepSeek-V4 Pro
Move tutorials above examples in website toctree per review. Co-authored-by: DeepSeek-V4 Pro
Remove hard line wrapping, drop redundant install step and list, move literalinclude to end, fix class structure wording, use json lexer for ipynb. Co-authored-by: DeepSeek-V4 Pro
Use actual solver class name in step 7 title, add step 8 for saving/loading results, remove images and execution metadata, fix codespell typo, format with black, normalize line endings. Co-authored-by: DeepSeek-V4 Pro
|
Hi @camUrban, thanks for the thorough review! I've addressed all the inline comments and pushed the fixes:
Also:
Could you please take another look when you have a moment? Re-requesting review now. Thanks! |
|
Just pushed a small fix: restored |
The docs site showed the notebook as raw JSON. Render it with myst-nb from committed outputs, with execution off because the docs build does not install the runtime dependencies. Copy the notebooks from tutorials/ into the Sphinx source tree at build time so tutorials/ stays the single source. Add scripts/execute_tutorials.py to regenerate the stored outputs. It hides the git repository from the kernel so the provenance warnings a dirty tree triggers stay out of the outputs, renders PyVista off screen, merges per-line stream outputs, and strips execution metadata. Save images to files and embed them from markdown cells instead of storing them as base64 outputs. Replace the per-panel Matplotlib plot with the draw function's render, since Panel may leave the public API, and drop the trailing bare-name lines that would store object reprs with memory addresses. Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
|
Hey @TianHengZhuang! I'm about to post specific review comments, but decided to first push a change to your branch, which changes how the notebook is rendered and adds a script that stores its outputs. Please pull before editing. The workflow for any notebook change is now: edit, run |
camUrban
left a comment
There was a problem hiding this comment.
Hi @TianHengZhuang. Here are my final comments. Great work again 😃
Could you mark each comment as "resolved" once you've completed it? After they are all resolved, feel free to request a final review.
Also, apologies for accidentally approving this before. I meant to click "Request changes". My bad 😆
|
By the way, don't worry about the Read the Docs check showing as failed. The build didn't get past fetching the SciPy intersphinx inventory because docs.scipy.org is shakey at the moment. It will pass once SciPy's docs are back and the build is retried. In the meantime, please check the site locally after each change. From the repository root, with your virtual environment active: pip install -r docs/website/requirements_docs.txt
sphinx-build -W --keep-going -b html docs/website docs/website/_build/htmlThen open |
7cb8113 to
c593da9
Compare
|
Thanks for the review, @camUrban! I've pushed a new commit that addresses all documentation comments:
The notebook content fixes from the previous commit are also in place. Ready for your review whenever you have time! |
|
Thanks for the detailed review! I've pushed a new commit (2d7966f) addressing all the documentation comments in docs/website/tutorials/getting_started.md, docs/website/tutorials.md, and docs/website/README.md, plus the inline notebook fixes in the previous commit (c593da9). Since the inline review comments can only be marked as resolved by the comment author via the GitHub UI, could you mark each one as resolved when you have a moment, or simply approve the PR if everything looks good? Either way works for me - I just want to make sure I'm not missing anything. |
A force push to the pull request branch replaced af96712 with two commits based on the previously reviewed head, so the rendering change and the author's newest edits diverged. Carry the author's four wording edits from c593da9 onto the rendered notebook and re-execute it, so the branch holds both. Co-authored-by: TianhengZhuang <39034691+TianHengZhuang@users.noreply.github.com>
2d7966f to
5e08b45
Compare
# Conflicts: # requirements_dev.txt
|
Hi @TianHengZhuang. Thanks for taking the time to look over my comments. Unfortunately, it looks like you force-pushed over my changes. Thankfully, I still had a local copy, so I was able to re-push them (with your edits merged in). ImportantPlease don't force push unless you are certain why a push is failing. If Instead, get in the habit of running Review CommentsThanks for letting me know that you can't resolve comments. I didn't know that! I've resolved the comments that you fixed. However, there are still eight that need to be taken care of. Let me know if you have any further questions. And, one last reminder, please Editing the NotebookThe notebook is now rendered on the documentation site from the outputs stored in the file, so there are a couple of extra steps after editing it:
Checking Your Work on the RTD WebsiteEvery push builds a preview of the documentation site. Click on the "docs/readthedocs.org:pterasoftware" check at the bottom of this PR, and open Tutorials in the sidebar. That is exactly what readers will see, rendered from your stored outputs, so it's the best way to check a change. You can also build it locally: pip install -r docs/website/requirements_docs.txt
sphinx-build -W --keep-going -b html docs/website docs/website/_build/htmland open |
Description
Add a "Getting Started" tutorial page and Jupyter notebook that walk new users through a first steady horseshoe vortex lattice method (VLM) simulation from scratch.
Motivation
New users currently have to piece together the object model from the API reference and the example scripts. A step-by-step tutorial lowers the barrier to entry, gives a concrete starting point for exploring the library, and makes Ptera Software easier to evaluate and adopt.
Relevant Issues
None.
Changes
tutorials/getting_started.ipynb, a runnable notebook that builds anAirfoil,WingCrossSection,Wing,Airplane, andOperatingPoint, runs a steady VLM simulation withSteadyHorseshoeVortexLatticeMethodSolver, and covers results, visualization, logging, and saving/loading.docs/website/tutorials/getting_started.md, which embeds the notebook on the documentation site.Dependency Updates
None.
Change Magnitude
Minor: Small change such as a bug fix, small enhancement, or documentation update.
Checklist (check each item when completed or not applicable)
mainand is up to date with the upstreammainbranch.--in-place --black). See the style guide for type hints and docstrings for more details.pterasoftwarepackage use type hints. See the style guide for type hints and docstrings for more details.testspackage.testspackage.ascii-only,pre-commit-hooks, andzizmorGitHub actions.lintjob of theCIGitHub action.testjobs of theCIGitHub action.Assisted-by: Marvis (AI assistant)