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

Simplify quadilateral_area + better doc #66

Merged
merged 1 commit into from
Sep 5, 2023

Conversation

navidcy
Copy link
Contributor

@navidcy navidcy commented Sep 5, 2023

I changed to human numbering 1, 2, 3,...

I think the formula a1 + a2 + a3 + a4 is exact so no need to update that. I did update the docs a bit to be truthful to what quadrilateral_area does. Have a look @ashjbarnes, thanks!

@navidcy
Copy link
Contributor Author

navidcy commented Sep 5, 2023

Together with #65 this PR resolves #39.

@codecov
Copy link

codecov bot commented Sep 5, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (1df6930) 15.42% compared to head (b36c123) 15.42%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #66   +/-   ##
=======================================
  Coverage   15.42%   15.42%           
=======================================
  Files           2        2           
  Lines         428      428           
=======================================
  Hits           66       66           
  Misses        362      362           
Files Changed Coverage Δ
regional_mom6/regional_mom6.py 14.89% <100.00%> (ø)

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

@navidcy navidcy requested a review from angus-g September 5, 2023 06:21
@ashjbarnes
Copy link
Collaborator

Looks good thanks Navid! Yeah this function is one I borrowed and was really opaque earlier

@navidcy navidcy merged commit 2f9d12b into main Sep 5, 2023
5 checks passed
@navidcy navidcy deleted the ncc/better-quadrilateral_area branch September 5, 2023 21:20
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.

2 participants