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

[doc] Update "Functions that Checking Units or Assigning Units" and Fix some typo #87

Merged
merged 3 commits into from
Dec 27, 2024

Conversation

Routhleck
Copy link
Collaborator

@Routhleck Routhleck commented Dec 27, 2024

This pull request includes updates to the brainunit/_unit_constants.py file to correct the scale factors for various units and updates to the docs/index.rst file to include new documentation links. The most important changes are as follows:

Corrections to unit scale factors:

  • brainunit/_unit_constants.py: Corrected the scale factors for pressure units (atmosphere, bar, torr, psi) to use pascal.scale instead of meter.scale.
  • brainunit/_unit_constants.py: Corrected the scale factor for the energy unit electronvolt to use joule.scale - 19 instead of -19.

Documentation updates:

  • docs/index.rst: Added new links to the documentation for linalg_functions.ipynb, fft_functions.ipynb, and lax_functions.ipynb.

Summary by Sourcery

Clarify the usage of functions for checking and assigning units, and add examples for using decorators.

Enhancements:

  • Improve unit scale factor accuracy for pressure and energy units.

Documentation:

  • Document the check_dims, check_units, and assign_units decorators with examples.

Copy link
Contributor

sourcery-ai bot commented Dec 27, 2024

Reviewer's Guide by Sourcery

This pull request corrects unit scale factors in brainunit/_unit_constants.py and adds documentation links to docs/index.rst.

Class diagram showing unit constant updates

classDiagram
    class Unit {
      +create(dim, name, dispname, scale, factor)
    }
    class PressureUnits {
      +atmosphere: Unit
      +bar: Unit
      +torr: Unit
      +psi: Unit
    }
    class EnergyUnits {
      +electron_volt: Unit
    }
    note for PressureUnits "Scale factors updated to use pascal.scale"
    note for EnergyUnits "Scale factor updated to use joule.scale - 19"
    Unit <|-- PressureUnits
    Unit <|-- EnergyUnits
Loading

File-Level Changes

Change Details Files
Corrected the scale factors for pressure units to use pascal.scale.
  • Changed the scale factor for atmosphere, bar, torr, and psi to use pascal.scale instead of meter.scale.
brainunit/_unit_constants.py
Corrected the scale factor for the electronvolt energy unit.
  • Changed the scale factor for electronvolt to use joule.scale - 19 instead of -19.
brainunit/_unit_constants.py
Added documentation links for new functions.
  • Added links for linalg_functions.ipynb, fft_functions.ipynb, and lax_functions.ipynb.
docs/index.rst
Updated the title of the "customize_functions.ipynb" notebook.
  • Changed the title from "Functions that Checking Units" to "Functions that Checking Units or Assigning Units".
docs/mathematical_functions/customize_functions.ipynb
Added import statements and examples for using the "check_units", "assign_units", and "check_dims" decorators.
  • Added examples for basic usage, validating return values, validating multiple return values, and validating dictionary return values for each decorator.
docs/mathematical_functions/customize_functions.ipynb
Updated the "customize_functions.ipynb" notebook to include detailed explanations and examples of using the "check_units", "assign_units", and "check_dims" decorators.
  • Added comprehensive documentation and examples to demonstrate the functionality and usage of each decorator, including how to handle different types of input and return values, and how to handle unit mismatches and errors.
docs/mathematical_functions/customize_functions.ipynb

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @Routhleck - I've reviewed your changes - here's some feedback:

Overall Comments:

  • Consider doing a final proofreading pass on the documentation text to fix minor typos and improve readability.
Here's what I looked at during the review
  • 🟡 General issues: 2 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

brainunit/_unit_constants.py Show resolved Hide resolved
brainunit/_unit_constants.py Show resolved Hide resolved
@chaoming0625 chaoming0625 merged commit 9063787 into main Dec 27, 2024
24 checks passed
@chaoming0625 chaoming0625 deleted the update-docs-241227 branch December 27, 2024 15:21
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.

2 participants