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

[FEA] Add Doxygen CI check #9373

Closed
jrhemstad opened this issue Oct 5, 2021 · 10 comments · Fixed by #11057
Closed

[FEA] Add Doxygen CI check #9373

jrhemstad opened this issue Oct 5, 2021 · 10 comments · Fixed by #11057
Assignees
Labels
doc Documentation feature request New feature or request libcudf Affects libcudf (C++/CUDA) code.

Comments

@jrhemstad
Copy link
Contributor

Is your feature request related to a problem? Please describe.

As #9355 shows, Doxygen documentation can easily get out of date.

No one likes out of date documentation. This leads to a bad experience for users and developers alike.

Describe the solution you'd like

Doxygen will generate warnings if the documentation does not match the code.

We should eliminate all current sources of warnings and then enable these warnings as a CI check to ensure Doxygen documentation stays up to date.

Additional context

This check should probably be optional at the beginning, but eventually should be required.

@jrhemstad jrhemstad added feature request New feature or request gpuCI libcudf Affects libcudf (C++/CUDA) code. labels Oct 5, 2021
@davidwendt
Copy link
Contributor

#9355 (comment)

@beckernick beckernick added the doc Documentation label Oct 27, 2021
@beckernick
Copy link
Member

Discussed offline with @jrhemstad . Automation to keep Doxygen documentation up-to-date is worthwhile but not likely to get done in 21.12.

@github-actions
Copy link

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

@karthikeyann
Copy link
Contributor

Number of warnings in Doxygen are high.
To get opinion,
some of trivial functions without documentation are operator<, operator=, has_nulls, begin, end, etc.
Should they be skipped or warning ignored or documented? (documentation for method, parameters and return type)

@harrism
Copy link
Member

harrism commented Dec 8, 2021

Seems to me that public APIs should be documented. For detail APIs, we definitely have the option to skip. Is there any way to auto-generate docs for standard functions like operator<, operator=?

@github-actions
Copy link

github-actions bot commented Jan 7, 2022

This issue has been labeled inactive-30d due to no recent activity in the past 30 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed. This issue will be labeled inactive-90d if there is no activity in the next 60 days.

@github-actions
Copy link

github-actions bot commented Apr 7, 2022

This issue has been labeled inactive-90d due to no recent activity in the past 90 days. Please close this issue if no further response or action is needed. Otherwise, please respond with a comment indicating any updates or changes to the original issue and/or confirm this issue still needs to be addressed.

@karthikeyann
Copy link
Contributor

Is there any way to auto-generate docs for standard functions like operator<, operator=?

CUB uses ALIASES. we could add aliases for these operators for ease the effort.

Another idea is to add custom filter command INPUT_FILTER.
I tested a simple example, for operator= as follows.

  • created filter.sh with content sed 's/^.* operator=(/\/** @brief assignment operator. @returns reference to object **\/ \0/g' $@
  • set INPUT_FILTER = /path/to/filter.sh in Doxyfile.
    All warnings related to operator= are gone.

how about we implement an input filter?
Note: As stated in https://www.doxygen.nl/manual/config.html#cfg_input_filter, we cannot insert or remove new lines.

Note that the filter must not add or remove lines; it is applied before the code is scanned, but not when the output code is generated. If lines are added or removed, the anchors will not be placed correctly.

@harrism
Copy link
Member

harrism commented May 12, 2022

Can the input filter only be applied to undocumented functions? Wouldn't want to overwrite existing ones.

Is there much value in doing it this way rather than fixing it once, manually? (Or running an automated fix just once) This would only fix the operators that we write the script to fix, and is only needed on public APIs. Does that account for a large fraction of the errors / warnings?

rapids-bot bot pushed a commit that referenced this issue May 14, 2022
This PR fixes 433 doxygen warnings.

As prerequisite for doxygen CI check #9373, the warnings from doxygen should be minimized as much as possible. 
This is one of the series of PRs to fix doxygen documentation warnings.

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - Nghia Truong (https://github.com/ttnghia)
  - AJ Schmidt (https://github.com/ajschmidt8)

URL: #10842
@karthikeyann
Copy link
Contributor

karthikeyann commented May 16, 2022

input filter processes the entire file. so, filter should identify undocumented functions.
Running an automated or manual fix once is preferred.

The warnings are distributed among 67 files. 1483 warnings.
Here are the files with most warnings.

  • 293 warnings from cudf/io/
  • column, scalar, table
  • strings, lists
  • utilities, cudf_test

rapids-bot bot pushed a commit that referenced this issue May 18, 2022
fixes part of #9373

Add missing documentation in scalar/ headers to fix doxygen warnings

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #10861
rapids-bot bot pushed a commit that referenced this issue May 24, 2022
Fixes parts of #9373
added missing documentation in types.hpp to fix doxygen warnings
fixes 30 warnings

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)
  - David Wendt (https://github.com/davidwendt)

URL: #10895
rapids-bot bot pushed a commit that referenced this issue May 27, 2022
Fixes parts of #9373
added missing documentation to fix doxygen warnings in multiple files

fixes 93 warnings.

Authors:
  - Karthikeyan (https://github.com/karthikeyann)
  - Vyas Ramasubramani (https://github.com/vyasr)

Approvers:
  - Vukasin Milovanovic (https://github.com/vuule)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #10913
rapids-bot bot pushed a commit that referenced this issue May 28, 2022
… files (#10912)

Fixes parts of #9373
added missing documentation to fix doxygen warnings in multiple files
- cpp/include/cudf/io/avro.hpp
- cpp/include/cudf/io/csv.hpp
- cpp/include/cudf/io/json.hpp
- cpp/include/cudf/io/orc.hpp
- cpp/include/cudf/io/orc_metadata.hpp
- cpp/include/cudf/io/parquet.hpp

fixes 194 warnings

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - Yunsong Wang (https://github.com/PointKernel)
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Nghia Truong (https://github.com/ttnghia)

URL: #10912
rapids-bot bot pushed a commit that referenced this issue May 28, 2022
Fixes parts of #9373
added missing documentation in aggregation.hpp to fix doxygen warnings
fixes 108 warnings.

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - Nghia Truong (https://github.com/ttnghia)
  - David Wendt (https://github.com/davidwendt)

URL: #10887
rapids-bot bot pushed a commit that referenced this issue May 28, 2022
Fixes parts of #9373
added missing documentation to fix doxygen warnings in multiple files cpp/include/cudf/*.hpp
fixes 40 warnings

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - David Wendt (https://github.com/davidwendt)
  - Bradley Dice (https://github.com/bdice)

URL: #10896
ttnghia pushed a commit to ttnghia/cudf that referenced this issue May 31, 2022
Fixes parts of rapidsai#9373
added missing documentation to fix doxygen warnings in multiple files cpp/include/cudf/*.hpp
fixes 40 warnings

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - David Wendt (https://github.com/davidwendt)
  - Bradley Dice (https://github.com/bdice)

URL: rapidsai#10896
rapids-bot bot pushed a commit that referenced this issue Jun 1, 2022
Fixes parts of #9373
added missing documentation to fix doxygen warnings in `structs/*.hpp` and `lists/*.hpp`
fixes 67 warnings.

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - David Wendt (https://github.com/davidwendt)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #10923
rapids-bot bot pushed a commit that referenced this issue Jun 1, 2022
…y/ headers (#10921)

Fixes parts of #9373
added missing documentation to fix doxygen warnings in multiple files
ast/, rolling, tdigest/, wrappers, dictionary/ headers

fixes 82 warnings.

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - Conor Hoekstra (https://github.com/codereport)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #10921
rapids-bot bot pushed a commit that referenced this issue Jun 1, 2022
Fixes parts of #9373
added missing documentation to fix doxygen warnings in `fixed_point/fixed_point.hpp`
fixes some existing docstrings.
fixes 47 warnings.

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)
  - David Wendt (https://github.com/davidwendt)

URL: #10922
rapids-bot bot pushed a commit that referenced this issue Jun 2, 2022
Fixes parts of #9373
added missing documentation to fix doxygen warnings in multiple files
ignored docs for `cudf::string_view::const_iterator`

fixes 78 warnings.

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - Conor Hoekstra (https://github.com/codereport)
  - David Wendt (https://github.com/davidwendt)

URL: #10937
rapids-bot bot pushed a commit that referenced this issue Jun 2, 2022
Fixes parts of #9373
added missing documentation to fix doxygen warnings in table headers
ignores doc generation for `strong_index_comparator_adapter`

fixes 166  warnings.

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - David Wendt (https://github.com/davidwendt)
  - Vyas Ramasubramani (https://github.com/vyasr)

URL: #10964
rapids-bot bot pushed a commit that referenced this issue Jun 3, 2022
Fixes parts of #9373
added missing documentation to fix doxygen warnings in multiple files cpp/include/cudf/utilities/
fixes 167 warnings

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - Ram (Ramakrishna Prabhu) (https://github.com/rgsl888prabhu)
  - David Wendt (https://github.com/davidwendt)

URL: #10974
rapids-bot bot pushed a commit that referenced this issue Jun 5, 2022
Fixes parts of #9373
added missing documentation to fix doxygen warnings in column, column_view, column_device_view, column factories headers

fixes 155 warnings.

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - Vyas Ramasubramani (https://github.com/vyasr)
  - Bradley Dice (https://github.com/bdice)

URL: #10963
@karthikeyann karthikeyann self-assigned this Jun 5, 2022
rapids-bot bot pushed a commit that referenced this issue Jun 6, 2022
Fixes parts of #9373
added missing documentation to fix doxygen warnings in multiple headers
cudf_test/, nvtext/, cudf_kafka/, ipc.hpp

With this PR, there are _zero doxygen warnings_!  ☺️

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - Yunsong Wang (https://github.com/PointKernel)
  - Nghia Truong (https://github.com/ttnghia)
  - David Wendt (https://github.com/davidwendt)

URL: #11003
rapids-bot bot pushed a commit that referenced this issue Jun 7, 2022
closes #9373
Add doxygen CI check to check warnings for missing documentation
Depends on PR #11056

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - Conor Hoekstra (https://github.com/codereport)
  - Nghia Truong (https://github.com/ttnghia)
  - Sevag Hanssian (https://github.com/sevagh)

URL: #11057
rapids-bot bot pushed a commit that referenced this issue Jun 20, 2022
- Adds doxygen-check pre-commit hook (verbose)
  - doxygen-check passes with warning if doxygen is not present
  - doxygen-check passes with warning if supported doxygen version 1.8.20 to 1.9.1 is not present
  - doxygen-check fails if any warning or error is thrown by doxygen
  - since pre-commit runs as part of style check, it's removed from _style.sh_
- updated Doxyfile configuration to work with versions 1.8.20 to 1.9.1
- added doxygen=1.8.20 to conda dev environment
- mentioned doxygen.sh in CONTRIBUTING.md guide
Related to #9373 (Add Doxygen CI check)

Authors:
  - Karthikeyan (https://github.com/karthikeyann)

Approvers:
  - Bradley Dice (https://github.com/bdice)
  - Yunsong Wang (https://github.com/PointKernel)
  - AJ Schmidt (https://github.com/ajschmidt8)

URL: #11076
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Documentation feature request New feature or request libcudf Affects libcudf (C++/CUDA) code.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants