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

Fix up weighted blending documentation #1961

Merged
merged 2 commits into from
Nov 10, 2023

Conversation

brhooper
Copy link
Contributor

@brhooper brhooper commented Oct 23, 2023

Addresses https://github.com/metoppv/mo-blue-team/issues/497

Description
Moves document which describes the weighted blending method into /improver/doc/files/ and updates the link to that document within the weighted blending doc-string to refer to the new location.

Testing:

  • Ran tests and they passed OK

@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (27531d9) 98.37% compared to head (4b82dce) 98.38%.
Report is 5 commits behind head on master.

❗ Current head 4b82dce differs from pull request most recent head 218b2f7. Consider uploading reports for the commit 218b2f7 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1961   +/-   ##
=======================================
  Coverage   98.37%   98.38%           
=======================================
  Files         123      124    +1     
  Lines       11942    11976   +34     
=======================================
+ Hits        11748    11782   +34     
  Misses        194      194           
Files Coverage Δ
improver/blending/weighted_blend.py 100.00% <ø> (ø)

... and 1 file with indirect coverage changes

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

Copy link
Contributor

@bayliffe bayliffe left a comment

Choose a reason for hiding this comment

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

A request.

@@ -269,8 +269,7 @@ class PercentileBlendingAggregator:

References:
Combining Probabilities by Caroline Jones, 2017:
https://github.com/metoppv/improver/files/1128018/
Combining_Probabilities.pdf
/improver/doc/files/Combining_Probabilities.pdf
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/improver/doc/files/Combining_Probabilities.pdf
:download:`Combining Probabilities by Caroline Jones, 2017 <../files/Combining_Probabilities.pdf>`

Could we make this downloadable in the external documentation with a link like this please?
The :download: bit makes readthedocs copy the file across to an accessible location. The path is relative to the source directory in the improver directory strucuture (improver/doc/source).

You cannot just commit this change as you need to delete the line above or it will have the title of the pdf twice.

You can see what that will look like here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for this - I've made the suggested change.

@brhooper brhooper removed their assignment Nov 6, 2023
@mspelman07 mspelman07 merged commit c9a148e into metoppv:master Nov 10, 2023
8 checks passed
MoseleyS pushed a commit to MoseleyS/improver that referenced this pull request Aug 22, 2024
* Add Combining_Probabilities.pdf to /doc/files. Update link in weighted blenidng doc-string to new location.

* Change following review - change document path to a download link in docstring.
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.

3 participants