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

Add doxygen for the assigned functions: H5Pregister1,H5Pinsert1,H5Pen… #352

Merged
merged 3 commits into from
Feb 19, 2021
Merged

Add doxygen for the assigned functions: H5Pregister1,H5Pinsert1,H5Pen… #352

merged 3 commits into from
Feb 19, 2021

Conversation

kyang2014
Copy link
Collaborator

…code1, H5Pget_filter_by_id1,H5Pget_version, H5Pset_file_space,H5Pget_file_space. Someone already adds H5Pget_filter1. Also fixs an extra parameter 'close' call back function for HPregister2.

…code1, H5Pget_filter_by_id1,H5Pget_version, H5Pset_file_space,H5Pget_file_space. Someone already adds H5Pget_filter1. Also fixs an extra parameter 'close' call back function for HPregister2.
@kyang2014 kyang2014 closed this Feb 18, 2021
@kyang2014
Copy link
Collaborator Author

Cannot choose the reviewers and see if I close and reopen it can do the trick.

@kyang2014 kyang2014 reopened this Feb 18, 2021
@lrknox lrknox requested review from bljhdf and gheber and removed request for qkoziol, soumagne, fortnern, jhendersonHDF and jrmainzer February 18, 2021 17:16
Copy link
Member

@gheber gheber left a comment

Choose a reason for hiding this comment

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

Great job. Let's eliminate a few repetitions if you have time! See the comments inline. Every repetition is the potential for inconsistency and we just had too much of that.

src/H5Ppublic.h Outdated
* given the opportunity to close any temporary data structures that were
* set up when the routine was called. The C++ application should save some
* state as the routine is started so that any problem that occurs might be
* diagnosed.
Copy link
Member

Choose a reason for hiding this comment

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

Let's not repeat this block literally everywhere! Would you mind appending the following single line to aliases?

ALIASES += cpp_c_api_note="\note \Bold{C++ Developers using HDF5 C-API functions beware:}\nIf a C routine that takes a function pointer as an argument is called from within C++ code, the C routine should be returned from normally. Examples of this kind of routine include callbacks such as H5Pset_elink_cb() and H5Pset_type_conv_cb() and functions such as H5Tconvert() and H5Ewalk2().\n Exiting the routine in its normal fashion allows the HDF5 C library to clean up its work properly. In other words, if the C++ application jumps out of the routine back to the C++ \c catch statement, the library is not given the opportunity to close any temporary data structures that were set up when the routine was called. The C++ application should save some state as the routine is started so that any problem that occurs might be diagnosed."

Then you can replace this paragraph by just \cpp_c_api_note

src/H5Ppublic.h Outdated
* given the opportunity to close any temporary data structures that were
* set up when the routine was called. The C++ application should save some
* state as the routine is started so that any problem that occurs might be
* diagnosed.
Copy link
Member

Choose a reason for hiding this comment

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

See the comment above.

src/H5Ppublic.h Outdated
* this property is being created. The #H5P_prp_create_func_t
* callback function is defined as follows:
*
* \snippet this H5P_prp_cb1_t_snip
Copy link
Member

Choose a reason for hiding this comment

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

Using snippets for callbacks is the way to go. Great! But we shouldn't repeat the parameter lists everywhere. Since Doxygen hyperlinks everything, the user has to just click on the callback type and will be taken to the documentation. The parameters should be defined once with the callback. As an example see H5E_walk2_t in H5Epublic.h around line 159. Does that make sense?

src/H5Ppublic.h Outdated
* the property. The #H5P_prp_set_func_t callback function is defined
* as follows:
*
* \snippet this H5P_prp_cb2_t_snip
Copy link
Member

Choose a reason for hiding this comment

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

Ditto. Let's not repeat parameter lists of callbacks.

src/H5Ppublic.h Outdated
* property value. The #H5P_prp_get_func_t callback function is
* defined as follows:
*
* \snippet this H5P_prp_cb2_t_snip
Copy link
Member

Choose a reason for hiding this comment

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

Ditto. Let's not repeat parameter lists of callbacks.

src/H5Ppublic.h Outdated
* The \p prp_set routine is called before a new value is copied
* into the property. The #H5P_prp_set_func_t callback function
* is defined as follows:
* \snippet this H5P_prp_cb2_t_snip
Copy link
Member

Choose a reason for hiding this comment

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

Ditto. Let's not repeat parameter lists of callbacks.

src/H5Ppublic.h Outdated
* a property value. The #H5P_prp_get_func_t callback function
* is defined as follows:
*
* \snippet this H5P_prp_cb2_t_snip
Copy link
Member

Choose a reason for hiding this comment

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

Ditto. Let's not repeat parameter lists of callbacks.

src/H5Ppublic.h Outdated
* deleted from a property list. The #H5P_prp_delete_func_t
* callback function is defined as follows:
*
* \snippet this H5P_prp_cb2_t_snip
Copy link
Member

Choose a reason for hiding this comment

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

Ditto. Let's not repeat parameter lists of callbacks.

src/H5Ppublic.h Outdated
*
* The #H5P_prp_copy_func_t callback function is defined as follows:
*
* \snippet this H5P_prp_cb1_t_snip
Copy link
Member

Choose a reason for hiding this comment

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

Ditto. Let's not repeat parameter lists of callbacks.

src/H5Ppublic.h Outdated
* property is being closed.
*
* The #H5P_prp_close_func_t callback function is defined as follows:
* \snippet this H5P_prp_cb1_t_snip
Copy link
Member

Choose a reason for hiding this comment

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

Ditto. Let's not repeat parameter lists of callbacks.

@@ -1582,16 +1582,12 @@ H5_DLL int H5Piterate(hid_t id, int *idx, H5P_iterate_t iter_func, void *iter_da
* property is being closed. The #H5P_prp_close_func_t callback
* function is defined as follows:
*
* \snippet this H5P_prp_cb2_t_snip
* \snippet this H5P_prp_cb1_t_snip
Copy link
Contributor

Choose a reason for hiding this comment

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

The 'cb1' / 'cb2' types are just convenience / maintenance helpers in the header file - the documentation should use the types for the parameters in the H5P* routines.

Copy link
Contributor

Choose a reason for hiding this comment

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

e.g. H5P_prp_create_func_t, H5P_prp_set_func_t, etc.

For Quincey's comments, since we are not supposed to change the source code.
I leave this to future improvements.
@lrknox lrknox merged commit 7a0443d into HDFGroup:doxygen2 Feb 19, 2021
lrknox added a commit that referenced this pull request Apr 26, 2021
* Fixed warnings and started H5Epublic.h.

* Include H5FD* headers to correctly resolve references.

* Doxygen2 (#330)

* H5Eauto_is_v2.

* Added a few more calls.

* Added a few more H5E calls.

* First cut of H5E v2.

* Added the deprecated v1 calls.

* Updated spacing.

* Once more.

* Taking some inspiration from Eigen3.

* Add doxygen for the assigned functions: H5Pregister1,H5Pinsert1,H5Pen… (#352)

* Add doxygen for the assigned functions: H5Pregister1,H5Pinsert1,H5Pencode1, H5Pget_filter_by_id1,H5Pget_version, H5Pset_file_space,H5Pget_file_space. Someone already adds H5Pget_filter1. Also fixs an extra parameter 'close' call back function for HPregister2.

* doxygen work. fixs format by using clang-format.

* doxgen work for H5Pregister1 etc. Addressed Barbara and Gerd's comments.
For Quincey's comments, since we are not supposed to change the source code.
I leave this to future improvements.

* added documentation for H5P APIs (#350)

* add documenation for H5Pget_buffer,H5Pget_data_transform,H5Pget_edc_check,H5Pget_hyper_vector_size,H5Pget_preserve,H5Pget_type_conv_cb,H5Pget_vlen_mem_manager,H5Pset_btree_ratios

* format corrections

* fixed grammer

* fixed herr_t

* Better name.

* A fresh look.

* add doxygen to H5Ppublic.h

* use attention instead of warning

* Add doxygen comments in H5Ppublic.h (#375)

* Add doxygen comments in H5Ppublic.h

* H5Pset_meta_block_size
* H5Pset_metadata_read_attempts
* H5Pset_multi_type
* H5Pset_object_flush_cb
* H5Pset_sieve_buf_size
* H5Pset_small_data_block_size
* H5Pset_all_coll_metadata_ops
* H5Pget_all_coll_metadata_ops

* Add DOXYGEN_EXAMPLES_DIR to src/CMakeLists.txt

* Fix clang-format errors

* Fix filenames in doxygen/examples

* add doxygen to H5Ppublic.h (#378)

* add doxygen to H5Ppublic.h

* use attention instead of warning

Co-authored-by: Kimmy Mu <kmu@hdfgroup.org>

* Revert "add doxygen to H5Ppublic.h (#378)"

This reverts commit 2ee1821.

* Updated Doxygen variables.

* I forgot to copy two images.

* Enable desktop search by default.

* Add my assigned Doxygen documentation.

* Remove whitespace at EOL.  Appease clang-format.

* Addressed Chris' comments.

* Added an alias for asynchronous functions.

* One space is enough for all of us.

* Slightly restructured RM page.

* address some issues

* reformatting

* Style external links.

* reformatting

* reformatting

* Added "Metadata Caching in HDF5" as a technical note example.

* Revise this soon!

* Added specification examples.

* Fixed references.

* Added H5AC cache image stuff and file format study.

* Added older FMT versions. Where did 1.0 go?

* Updated C/C++ note and replaced ambiguous labels.

* Reformat source with clang v10.0.1.

* Added the VFL technical note.

* Added what I believe might be called version 1.0 of the format.

* Added the remaining specs.

* Added H5Z callback documentation and fixed a few mistakes.

* Added dox for deprecated H5G calls and fixed a few snippet blockIDs.

* clang-format happy?

* Ok?

* Bonus track: Deprecated H5D functions.

* Carry over the more detailed group description.

* Added documentation for the missing and deprecated H5R calls.

* Life is easier and less repetitive w/ snippets. Use them!

* Eliminate the snippet block ID artifacts in the HTML rendering.

* Fixed snippet HTML artifacts and added a few missing calls.

* Under 20 H5Ps to go!

* Almost complete!

* "This is a form of pedantry up with which I will not put." (Churchill)

* Let's not waste as much space on bulleted lists!

* First complete (?) draft of the Doxygen-based RM.

* Completeness check and minor fixes along the way.

* Pedantry.

* Adding missing H5FD calls checkpoint.

* Pedantry.

* More pedantry.

* Added H5Pset_fapl_log.

* First draft of H5ES.

* Fixed warnings.

* Prep. for map module.

* First cut of the map module.

* Pedantry.

* Possible H5F introduction.

* Fix the indentation.

* Pedantry.

* Ditto.

* Thanks to the reviewers for their comments.

* Added missing images.

* Line numbers are a distraction here.

* More examples, references, and clean-up. Don't repeat yourself!

* Clang pedantry.

* Ditto.

* More reviewer comments...

* Templatized references and cleaned up \todos.

* Committing clang-format changes

* Fixed MANIFEST.

* Addressed Quincey's comments. (OCPLs)

* Fixed a few more \todo items.

* Fixed more \todo items.

* Added attribute life cycle.

* Forgot the examples file.

* Committing clang-format changes

* Pedantry.

* Live and learn!

* Added a sample H5D life cycle.

* Committing clang-format changes

* Pedantry.

Co-authored-by: kyang2014 <kyang2014@users.noreply.github.com>
Co-authored-by: Scot Breitenfeld <brtnfld@hdfgroup.org>
Co-authored-by: Kimmy Mu <kmu@hdfgroup.org>
Co-authored-by: Christopher Hogan <ChristopherHogan@users.noreply.github.com>
Co-authored-by: jya-kmu <53388330+jya-kmu@users.noreply.github.com>
Co-authored-by: David Young <dyoung@hdfgroup.org>
Co-authored-by: Larry Knox <lrknox@hdfgroup.org>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
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.

4 participants