Add docs for histogram #503

codejaeger · 2020-06-26T21:24:47Z

Description

Add documentation for the histogram functionality provided in PR #499 .

References

Blocked by Add histogram class and related functionality #499

Tasklist

Add test case(s)
Ensure all CI builds pass - RIP Travis CI, replaced with GitHub Actions
Review and approve

mloskot

It looks very good!
Quick review of the docs structure:

No need for core/ subdirectory, just move the files one level up, and remove the directory. The name core is an informal one, that, there is no physical structure named core.
The `extension/' directory does make sense, please keep it.
Please, remove _a_ infix and _it suffix from file names. Simple names like create.rst, fill.rst, subhistogram.rst are clear enough.

- Made changes suggested in first review - Add docs for relevant files

mloskot

Nitpick: The file name cumulativehistogram.rst should read cumulative.rst. It already lives in histogram/ directory.

mloskot

@codejaeger Well done! It's great to see GSoC features covered with documentation :-) I'm happy to approve this PR. No objections from you @lpranam ?

I have offered a few comments, but those are not show-stoppers.
Technical writing is not a trivial activity. We can always improve and refine documentation in future.

I'd suggest we merge it after we merge PR #499.
Next time you may prefer to stick docs in the same PR as implementation and tests.

doc/histogram/create.rst

doc/histogram/cumulative.rst

doc/histogram/extend.rst

doc/histogram/fill.rst

mloskot · 2020-07-26T20:04:16Z

doc/image_processing/contrast_enhancement/histogram_equalization.rst

+**Grayscale Image**
+
+.. figure:: barbara.png
+    :width: 600px
+    :align: center
+    :height: 300px
+    :alt: Could not load image.
+    :figclass: align-center
+
+**RGB**
+
+.. figure:: church.png
+    :width: 600px
+    :align: center
+    :height: 250px
+    :alt: Could not load image.
+    :figclass: align-center


I do not like the idea of storing 7MB files in GIL's repo.
Unfortunately, I can not offer an ideal alternative yet.

I'd suggest a temporary solution:

Remove the images from this PR

Paste the images as comments to this PR

Replace filename with URLs from GitHub

.. figure:: https://user-images.githubusercontent.com/12345/12811123-3877a980-8375-11ea-9960-1b88a3f9f87b.png ...

The thing is, once we start adding big binary files to Git repo, then even if we find an optimal solution for hosting images for docs, we will not be able to get rid of them easily, from Git history.

I have (slowly) started collecting externally hosted test images in dedicated repo https://github.com/boost-gil/test-images, so I perhaps we could have examples/ folder there and link from docs to there.

What do you think @lpranam ?

In my opinion, in docs we don't have to use the full-size images. Docs just need to demonstrate what our algorithms can do and do not need to show the exact accurate images, we can use highly compressed and small resolution images in docs. Otherwise what @mloskot suggested the great. I have no other objections regarding this PR.

@codejaeger Well done! It's great to see GSoC features covered with documentation :-) I'm happy to approve this PR.

Thanks a lot @mloskot : )

I'd suggest we merge it after we merge PR #499.
Next time you may prefer to stick docs in the same PR as implementation and tests.

Yes sure. Also this PR contains docs for the algorithm histogram equalization. I haven't pushed it yet since the histogram class is a dependency for it. Should I remove the docs for the algorithm?

I have compressed the images, they now occupy about 34 KB and 24 KB. I can also do what @mloskot suggested if needed.

though it is compressed for doc would be great to add real one on that repo so we can have those images for later.

@codejaeger If you want to try the idea outlined earlier, you should have now the write access to https://github.com/boost-gil/test-images Feel free to create doc/ directory (and any necessary subfolders) and put your images in there.

@codejaeger Could you add full size/resolution images to the test-images repo as @lpranam suggested?

@codejaeger Ping ☝️

Yep doing it. Thanks for the reminder, sorry I had totally forgotten about it.

mloskot · 2021-01-24T13:12:54Z

@codejaeger I've just git merge-updated your docs_for_histogram branch from the latest develop

codecov · 2021-03-24T14:32:03Z

Codecov Report

Merging #503 (b09a0c2) into develop (0c0fe1a) will increase coverage by 0.78%.
The diff coverage is 98.35%.

@@             Coverage Diff             @@
##           develop     #503      +/-   ##
===========================================
+ Coverage    77.81%   78.59%   +0.78%     
===========================================
  Files          110      117       +7     
  Lines         4367     5003     +636     
===========================================
+ Hits          3398     3932     +534     
- Misses         969     1071     +102

* Add docs for histogram * Add docs and make changes: - Made changes suggested in first review - Add docs for relevant files * Rename docs file and correct typos * Change doc for cumulative histogram * Add docs for histogram equalization * Make changes suggested in review * Add docs * Remove docs for algorithms * Move images to test_images Co-authored-by: Mateusz Łoskot <[email protected]>

codejaeger marked this pull request as draft June 26, 2020 21:25

Add docs for histogram

b3a453d

codejaeger force-pushed the docs_for_histogram branch from 48e083c to b3a453d Compare June 26, 2020 21:27

mloskot requested changes Jun 26, 2020

View reviewed changes

Add docs and make changes:

7da2aa4

- Made changes suggested in first review - Add docs for relevant files

codejaeger requested a review from mloskot June 29, 2020 16:03

codejaeger marked this pull request as ready for review June 29, 2020 16:03

codejaeger mentioned this pull request Jun 30, 2020

Add histogram class and related functionality #499

Merged

4 tasks

mloskot added cat/documentation google-summer-of-code labels Jun 30, 2020

mloskot requested changes Jun 30, 2020

View reviewed changes

Rename docs file and correct typos

3739b43

codejaeger requested a review from mloskot July 1, 2020 08:27

codejaeger added 2 commits July 5, 2020 19:06

Change doc for cumulative histogram

d7233a6

Add docs for histogram equalization

909dd83

codejaeger force-pushed the docs_for_histogram branch from be002b4 to 909dd83 Compare July 25, 2020 20:19

mloskot approved these changes Jul 26, 2020

View reviewed changes

Make changes suggested in review

79ca82f

codejaeger requested review from lpranam and mloskot July 31, 2020 09:33

codejaeger and others added 3 commits August 24, 2020 21:47

Add docs

f964999

Remove docs for algorithms

2fd0812

Merge branch 'develop' into docs_for_histogram

26d1d73

mloskot added this to the Boost 1.77+ milestone Mar 24, 2021

Move images to test_images

b09a0c2

codejaeger mentioned this pull request Mar 24, 2021

Update image url links for docs for histogram #586

Merged

2 tasks

lpranam approved these changes Mar 24, 2021

View reviewed changes

mloskot approved these changes Mar 24, 2021

View reviewed changes

mloskot merged commit dc9ba74 into boostorg:develop Mar 24, 2021

mloskot mentioned this pull request May 12, 2022

Prepare for release with Boost 1.80 #667

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add docs for histogram #503

Add docs for histogram #503

codejaeger commented Jun 26, 2020 •

edited by mloskot

Loading

mloskot left a comment

mloskot left a comment

mloskot left a comment •

edited

Loading

mloskot Jul 26, 2020

lpranam Jul 28, 2020

codejaeger Jul 31, 2020

codejaeger Jul 31, 2020 •

edited

Loading

lpranam Jul 31, 2020

mloskot Aug 3, 2020

mloskot Jan 24, 2021

mloskot Mar 24, 2021

codejaeger Mar 24, 2021 •

edited

Loading

mloskot commented Jan 24, 2021

codecov bot commented Mar 24, 2021

Add docs for histogram #503

Add docs for histogram #503

Conversation

codejaeger commented Jun 26, 2020 • edited by mloskot Loading

Description

References

Tasklist

mloskot left a comment

Choose a reason for hiding this comment

mloskot left a comment

Choose a reason for hiding this comment

mloskot left a comment • edited Loading

Choose a reason for hiding this comment

mloskot Jul 26, 2020

Choose a reason for hiding this comment

lpranam Jul 28, 2020

Choose a reason for hiding this comment

codejaeger Jul 31, 2020

Choose a reason for hiding this comment

codejaeger Jul 31, 2020 • edited Loading

Choose a reason for hiding this comment

lpranam Jul 31, 2020

Choose a reason for hiding this comment

mloskot Aug 3, 2020

Choose a reason for hiding this comment

mloskot Jan 24, 2021

Choose a reason for hiding this comment

mloskot Mar 24, 2021

Choose a reason for hiding this comment

codejaeger Mar 24, 2021 • edited Loading

Choose a reason for hiding this comment

mloskot commented Jan 24, 2021

codecov bot commented Mar 24, 2021

Codecov Report

codejaeger commented Jun 26, 2020 •

edited by mloskot

Loading

mloskot left a comment •

edited

Loading

codejaeger Jul 31, 2020 •

edited

Loading

codejaeger Mar 24, 2021 •

edited

Loading