Replaced isoareal quotient formula with sqrt of isperimetric quotient…#405
Conversation
…; docstring improvements
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #405 +/- ##
=======================================
- Coverage 82.7% 82.7% -0.0%
=======================================
Files 27 27
Lines 3829 3828 -1
=======================================
- Hits 3166 3165 -1
Misses 663 663
🚀 New features to boost your workflow:
|
|
If you install these to your env, then you should be able to build the docs by navigating to |
|
When I run |
|
I normally install editable versions of packages to my environment. pip install . -e |
Had to change to: pip install -e .which worked. Docs compiled successfully (I didn't break anything) and the equations rendered correctly. |
|
I've just got one change, and then it's all fine w/ me! |
Co-authored-by: Levi John Wolf <levi.john.wolf@gmail.com>
| With some manipulation, :math:`IAQ` can also be expressed as the square root of the Isoperimetric quotient, given by | ||
| implemented as `numpy.sqrt(isoperimetric_quotient(collection))`. Importantly, this means | ||
| that the :math:`IAQ` and :math:`IPQ` will rank shapes identically. |
There was a problem hiding this comment.
I know you're likely aware, but this sentence now reads as "... given by implemented as...". Just flagging this so it does not get merged.
There was a problem hiding this comment.
Yes, I'm aware. Thanks for bringing to my attention. I'm not used to editing files directly in GitHub. @ljwolf had proposed an edit, and I thought it would be easier to accept the edit and then add my own. I wasn't sure what would happen to his proposal if I edited the file in the same area.
Let me know if I should do this differently.
There was a problem hiding this comment.
It is fine like this if there's a follow-up commit. His proposal is just a comment that Github can turn into a commit. So if you did push your own without accepting, his suggestion would never make it to the commit history.
Added mathematical expression for Isoperimetric Area Quotient (IAQ) and clarified its relationship with Isoperimetric Quotient (IPQ).
|
@ljwolf I have updated the documentation for |
|
@leehach – those new failures are due to |
|
OK, everything is good to go, but I don't have merge power. Please merge when ready. (Sorry for accidental close and reopen, I thought it would be a combined merge/close PR.) |
Proposed fixes to #403 .
Commit includes the following changes:
_cast(). Replaces nested if; if; else with if;elif;else.isoareal_quotient()with square root ofisoperimetric_quotient(). Tested against builtin example datavirginia, and the new version yields the same result.isoperimetric_quotient()while providing LaTeX math formulas for both functions.I have tested the code, but regarding 3, I do not have my environment set up for producing documentation. I will follow up with someone on how to do that, but if a maintainer already has that set up and wants to check that the docstrings work, and in particular the math equations, that would be great.