Adding a Dask best practices section to the user guide#5190
Conversation
Codecov ReportPatch and project coverage have no change.
Additional details and impacted files@@ Coverage Diff @@
## main #5190 +/- ##
=======================================
Coverage 89.37% 89.37%
=======================================
Files 89 89
Lines 22419 22419
Branches 5380 5380
=======================================
Hits 20036 20036
Misses 1637 1637
Partials 746 746 ☔ View full report in Codecov by Sentry. |
lbdreyer
left a comment
There was a problem hiding this comment.
Looking good, thanks @HGWright !
A few general comments:
-
There are a few terms that need tidying up. I think we were quite lazy in the draft Dask best practices docs, but we need to be more careful here if this is to be included in the Iris docs, particularly, the following need to be update:
numpy->NumPy
netcdf->netCDF(I don't believe this needs to be capitalised)
Alsodaskshould always be capitalisedDask -
We have used
CPU'sin quite a few places, but the apostrophe is incorrect, so that should just beCPUs. -
You have used the term "multiprocessing system", but I think a term like "computing cluster" would be more appropriate.
-
There are a few examples of MO specific sections that need generalising a bit more, I have added specific comments where this is required.
lbdreyer
left a comment
There was a problem hiding this comment.
This is looking very close to being ready to merge!
There are just two outstanding issues that I can see:
- Malformed tables causing docs tests to fail. I suspect this could just be solved by adding the extra spaces that you lost when you changed
CPU's->CPUS. See my suggestions below: - "This branch is out-of-date with the base branch" I've never seen this error before. It might be alluding to a merge conflict? But maybe something else!
This is not an error. |
|
How open are we to further modifying this now? |
Some random things I have noticed as I re-read it (but we can still address afterwards/elsewhere):
|
I'd been in support of improving things before this gets added to a release. Are we intending this to go in Iris 3.5? |
I had thought so, but I see it's not actually on the board. |
Co-authored-by: lbdreyer <lbdreyer@users.noreply.github.com>
|
Great work @HGWright ! Please remember to create a new issue to address these points |
🚀 Pull Request
Description
I have brought the internal Dask best practice advice and examples into the Dask documentation. I have updated to change specific internal information to more generic language, more relevant in the documentation.
This should be linked to #4959. But does not fully close the issue.
Consult Iris pull request check list