Add critical_density
#1664
Add critical_density
#1664
Conversation
github-actions
bot
added
the
plasmapy.formulary
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thank you for making your first contribution to PlasmaPy! The project's future depends on contributors like you, so we deeply appreciate it! 🌱
To make sure that you get credit for this pull request (PR), please add yourself to the list of contributors in docs/about/credits.rst.
We encourage you to check out our contributor guide, which discusses topics like adding changelog entries. If you'd like to talk with us in real time, the easiest way is via our chat room. We also invite you to stop by our weekly community meeting or office hours.
The bottom of this page includes several checks that are run for every PR. Don't worry if something broke! We break stuff all the time. Eventually we'll want to get checks on this PR to pass before merging it. Clicking on Details next to the check will show the results of the test, including error messages. Please feel free to ask for help if you're having trouble (we do that all the time, as well).
Our testing guide describes these checks in more detail, as well as how to write and run tests. We recommend trying out test-driven development: write tests first, and then write code to make tests pass. Remember to run tests often!
As described in our documentation guide, PlasmaPy's documentation is written in reStructuredText using the numpydoc docstring standard. To see a preview of the documentation from this PR, click on Details next to the docs/readthedocs.org:plasmapy check.
To automagically fix most linter problems, comment on this PR with pre-commit.ci autofix, followed by a git pullto bring the changes home to your own computer.
A code maintainer should come by soon to begin a code review. This step often includes suggestions for fixing bugs, adding tests, or simplifying the implementation. After that you'll have a chance to make changes. If you'd like more time before a code review happens, you can convert this PR to a draft now and mark it as ready for review later.
Finally, we try to be the best community we can be. We have a Code of Conduct to foster a community of care and appreciation.
We thank you once again! ✨
Codecov Report
@@ Coverage Diff @@
## main #1664 +/- ##
==========================================
- Coverage 97.23% 97.22% -0.01%
==========================================
Files 84 85 +1
Lines 8016 8012 -4
==========================================
- Hits 7794 7790 -4
Misses 222 222
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
pheuer
left a comment
pheuer
left a comment
There was a problem hiding this comment.
Nice start! I've left some comments on the code. The next step (after creating the densities.py file) will be to create tests using Pytest. You should create a /formulary/tests/test_densities.py file (look at test_frequenices or test_speeds for ideas). Some ideas for tests would be:
- Use pytest.raises() and pytest.warns() to check that invalid inputs raise the correct exceptions and warnings.
- Manually compute 1-2 values of n_c and write an assert test that checks that your function returns the correct value. That would look something like:
assert np.isclose(n_c, true_value)
Add `densities.py` & move `plasma_critical_density()` from `frequencies.py` to `densities.py` Revise changelog entry
|
So you're getting a CI error for the new module: (you can see that by clicking 'details' next to the failing test) I think you need to also add a link to this file in \docs\formulary\index.rst. That tells autodoc where to include this file's information in the overall structure of the documentation. |
Separate tests Add `densities` to toctree
pheuer
left a comment
pheuer
left a comment
There was a problem hiding this comment.
Very nice! Just a couple more minor comments and I think this is ready to merge.
Add relevant example to docstring Clarify docstring
pheuer
left a comment
pheuer
left a comment
There was a problem hiding this comment.
Thanks @JaydenR2305 ! This is really close - just a couple more notes and I think it's ready to go.
Clean up imports
Hey Peter, I need someone with write access to merge my pull request. Thank you for your patience throughout this process! |
|
@JaydenR2305 oops, yes I guess so. Ok, merged! |
|
And no problem at all - this was an excellent first contribution and I'm happy to help you with any more you wish to work on in the future! |
|
|
||
|
|
||
| class TestCriticalDensity: | ||
| """Test the plasma_critical_density function in densities.py.""" |
3 participants
Resolves PlasmaPy/hack-week#26
docstrings.