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

Improve documentation for base simulation classes #1295

Merged
merged 36 commits into from Apr 2, 2024

Conversation

ghwilliams
Copy link
Contributor

@ghwilliams ghwilliams commented Oct 16, 2023

This is another step toward improving the documentation of the API.
This time I only worked on a single file (simulation.py).
I think that these changes make the documentation better, but by no means I consider it the best that can be done. So, it is a work in (eternal) progress.

Summary

Summary added before Squash and Merge:

Extend documentation for base simulation classes and in SyntheticData class. Fix typo and rst style in mesh_utils.py. Add pymatsolver to the list of intersphinx_mapping in Sphinx's docs/conf.py.

PR Checklist

  • If this is a work in progress PR, set as a Draft PR
  • Linted my code according to the style guides.
  • Added tests to verify changes to the code.
  • Added necessary documentation to any new functions/classes following the
    expect style.
  • Marked as ready for review (if this is was a draft PR), and converted
    to a Pull Request
  • Tagged @simpeg/simpeg-developers when ready for review.

Reference issue

What does this implement/fix?

Additional information

But I would like in a very near future to look
closer at PDESimulation classes.
@dccowan dccowan self-assigned this Nov 28, 2023
Copy link

codecov bot commented Nov 28, 2023

Codecov Report

Attention: Patch coverage is 87.50000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 82.37%. Comparing base (489851f) to head (c762ce9).

Files Patch % Lines
SimPEG/simulation.py 87.50% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1295   +/-   ##
=======================================
  Coverage   82.37%   82.37%           
=======================================
  Files         166      166           
  Lines       25379    25379           
=======================================
  Hits        20905    20905           
  Misses       4474     4474           

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

@dccowan dccowan added this to the 0.21.0 milestone Dec 6, 2023
@dccowan dccowan requested a review from a team December 6, 2023 18:50
Copy link
Member

@santisoler santisoler left a comment

Choose a reason for hiding this comment

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

Thanks for this work @ghwilliams!

I think this is ready to be merged. I just left a few minor suggestions, mainly improving rst and LaTeX syntax in the docstrings.

Let me know what do you think.

SimPEG/data.py Outdated Show resolved Hide resolved
SimPEG/data.py Outdated Show resolved Hide resolved
SimPEG/simulation.py Outdated Show resolved Hide resolved
SimPEG/simulation.py Outdated Show resolved Hide resolved
SimPEG/simulation.py Outdated Show resolved Hide resolved
SimPEG/simulation.py Outdated Show resolved Hide resolved
SimPEG/simulation.py Outdated Show resolved Hide resolved
SimPEG/simulation.py Outdated Show resolved Hide resolved
SimPEG/simulation.py Outdated Show resolved Hide resolved
SimPEG/simulation.py Outdated Show resolved Hide resolved
@ghwilliams
Copy link
Contributor Author

@santisoler @dccowan Done all the suggested changes!

@santisoler santisoler self-requested a review January 15, 2024 19:38
@santisoler santisoler self-requested a review March 6, 2024 18:25
Copy link
Member

@santisoler santisoler left a comment

Choose a reason for hiding this comment

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

Hi @ghwilliams! Sorry for the delayed review of this PR. Thanks for applying the suggestions I made.

I just applied a few more that weren't resolved yet (no worries 🙂) and a some small changes to make flake8 happy. I also updated the branch against the latest changes in main and solved a few conflicts in simulation.py.

I left a little comment regarding the docstring of ExponentialSinusoidSimulation. Beside that, I think this is ready to be merged!

SimPEG/simulation.py Show resolved Hide resolved
Copy link
Member

@lheagy lheagy left a comment

Choose a reason for hiding this comment

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

Thanks so much @ghwilliams and @santisoler for your work on this pr! It is awesome to see improvements to the docs. I have just a few minor inline comments on the pr. Feel free to let me know if anything is unclear

SimPEG/simulation.py Outdated Show resolved Hide resolved
SimPEG/simulation.py Show resolved Hide resolved
SimPEG/simulation.py Outdated Show resolved Hide resolved
SimPEG/simulation.py Outdated Show resolved Hide resolved
SimPEG/simulation.py Outdated Show resolved Hide resolved
SimPEG/simulation.py Outdated Show resolved Hide resolved
SimPEG/simulation.py Outdated Show resolved Hide resolved
SimPEG/simulation.py Outdated Show resolved Hide resolved
SimPEG/simulation.py Outdated Show resolved Hide resolved
SimPEG/simulation.py Outdated Show resolved Hide resolved
ghwilliams and others added 9 commits March 28, 2024 16:04
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
Co-authored-by: Lindsey Heagy <lindseyheagy@gmail.com>
SimPEG/simulation.py Outdated Show resolved Hide resolved
SimPEG/data.py Outdated Show resolved Hide resolved
SimPEG/data.py Outdated Show resolved Hide resolved
@santisoler santisoler changed the title Doc base simulations Improve documentation for base simulation classes Apr 2, 2024
@santisoler
Copy link
Member

This is looking great @ghwilliams! Thanks again for all the effort you put into this, and thanks @lheagy for the reviews. I'll merge it after CI passes.

@santisoler santisoler merged commit eeb406f into simpeg:main Apr 2, 2024
16 checks passed
thibaut-kobold added a commit to KoBoldMetals/simpeg that referenced this pull request May 8, 2024
…al_update_21_release

* commit '07aeb65de865cd9df55e2d51ca840ea9d60f2234': (67 commits)
  Fix rst syntax in release notes for v0.21.0 (simpeg#1434)
  Replace use of `refine_tree_xyz` in DCIP tutorials (simpeg#1381)
  Add new Issue template for making a release (simpeg#1410)
  Safely run reviewdog on `pull_request_target` events (simpeg#1427)
  Use reviewdog to annotate PR's with black and flake8 errors. (simpeg#1424)
  Remove the parameters argument from docstring (simpeg#1417)
  Add release notes for v0.21.1 (simpeg#1416)
  Fix hard dask dependency (simpeg#1415)
  Publish documentation on azure (simpeg#1412)
  Add release notes for SimPEG v0.21 (simpeg#1409)
  Bump Black version to 24.3.0 (simpeg#1403)
  Remove link to "twitter" (simpeg#1406)
  Add Ying and Williams to AUTHORS.rst (simpeg#1405)
  Dask MetaSim (simpeg#1199)
  Update year in LICENSE (simpeg#1404)
  Update AUTHORS.rst (simpeg#1259)
  Minor adjustments to Sphinx configuration (simpeg#1398)
  Enforce regularization `weights` as dictionaries (simpeg#1344)
  Improve documentation for base simulation classes (simpeg#1295)
  Add link to User Tutorials to navbar in docs (simpeg#1401)
  ...

# Conflicts:
#	SimPEG/electromagnetics/time_domain/receivers.py
#	SimPEG/utils/__init__.py
#	SimPEG/utils/model_utils.py
#	tests/base/regularizations/test_full_gradient.py
#	tests/base/regularizations/test_regularization.py
#	tests/base/test_model_utils.py
#	tests/base/test_utils.py
#	tests/pf/test_forward_Mag_Linear.py
@ghwilliams ghwilliams deleted the Doc_BaseSimulations branch May 16, 2024 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants