Documentation reorganization - a suggestion #256
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #256 +/- ##
=======================================
Coverage 94.34% 94.34%
=======================================
Files 13 13
Lines 1697 1697
=======================================
Hits 1601 1601
Misses 96 96
☔ View full report in Codecov by Sentry. |
| modules = [ImageFiltering, OffsetArrays, Kernel, KernelFactors, ImageFiltering.MapWindow], | ||
| format = format, | ||
| sitename = "ImageFiltering", | ||
| pages = [ |
There was a problem hiding this comment.
Just a glance of the file changes:
- It is recommended to preserve existing spaces for readability while adding new lines.
- Please remember to add
.DS_Storeto the.gitignorefile to avoid unnecessary commits.
| Pad(style::Symbol, lo::Dims{N}, hi::Tuple{}) where {N} = Pad(style, lo, ntuple(d->0,Val(N))) | ||
| Pad(style::Symbol, lo::Tuple{}, hi::Dims{N}) where {N} = Pad(style, ntuple(d->0,Val(N)), hi) | ||
| Pad(style::Symbol, lo::Dims{N}, hi::Tuple{}) where {N} = Pad(style, lo, ntuple(d -> 0, Val(N))) | ||
| Pad(style::Symbol, lo::Tuple{}, hi::Dims{N}) where {N} = Pad(style, ntuple(d -> 0, Val(N)), hi) |
There was a problem hiding this comment.
Thanks for the commit. The following is my personal view, please fell free to correct me.
30 file changes in one commit might require more careful attention during code review. Additionally, some of these changes appear to be related to formatting strings. Perhaps we can split this into two separate changes: one for adding a new feature and another for changing the format.
Nonetheless, the file appears to be generally in good condition.
|
Many thanks for taking on this big task. I managed to build the documentation locally without any issues. There's obviously a lot to read through, but my first impression is that people will find the REPL experience much more pleasant now that you moved the LaTeX out into separate documents. The tutorial you wrote is also really nice, and in general the layout of the documentation feels like a great improvement. One thing I notice is that |
|
Can we please move this forward?It would be great to get this done, issa common gripe 😓 |
| convolution = imfilter(f,reflect(w),Fill(0,w)) | ||
| ``` | ||
|
|
||
| ## Miscellaneous border padding options |
There was a problem hiding this comment.
Is this section redundant with the one above?
Best way to make that happen is to give it a thorough review! But you could wait until my comments above are addressed. |
Would adding |
Co-authored-by: Tim Holy <[email protected]>
Co-authored-by: Tim Holy <[email protected]>
|
@timholy Thanks so much for the comments and review, and for taking the time... I confess I don't understand the fine details of the package, but perhaps the structure of the docs is a bit better now, and in the future will allow people to add more information in the right place. |
|
When I'm back from travels I'll build this locally and give it a read-through; assuming nothing serious comes up I'll merge as-is and we can fix minor problems later. Or if anyone else wants to build it and do that for me, just let me know the verdict and we can merge more quickly. |
|
I built it locally, and it looks awesome. Thanks ever so much, @cormullion! A huge contribution! |
|
Did these docs ever get released? |
This PR comprises a suggested reorganization of the documentation. My approach is probably reasonably similar to the Divio style - at least, try to keep different types of information separate, so that novices, casual users, experienced users, and package developers don't trip over material not meant for them. (I think this is probably the opposite of Images' current approach (judging from this RFC issue.)
The main thing was just to add some hierarchy to the manual, then to move as much stuff out of the docstrings as possible (cf my previous pet gripe, all that LaTeX). 😂
(I'm not too sure how you'd preview this...)