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

[docs] fixed and enhanced format of C API #2195

Merged
merged 2 commits into from May 28, 2019
Merged

[docs] fixed and enhanced format of C API #2195

merged 2 commits into from May 28, 2019

Conversation

StrikerRUS
Copy link
Collaborator

* \brief Create an empty dataset by sampling data.
* \param sample_data Sampled data, grouped by the column
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This lost param was the reason why #2076 ruined all C API docs.
It's quite strange, but \fn directive makes Doxygen produce xml without descriptions instead of generating the error.
Also, Doxygen's docs say that we don't need to use \fn directives when comments are placed directly before function:

Warning
Do not use this command if it is not absolutely needed, since it will lead to duplication of information and thus to errors.
http://www.doxygen.nl/manual/commands.html#cmdfn

So I removed them all.

LIGHTGBM_C_EXPORT int LGBM_DatasetCreateFromFile(const char* filename,
const char* parameters,
const DatasetHandle reference,
DatasetHandle* out);

/*!
* \fn LGBM_DatasetCreateFromSampledColumn
* \brief Create an empty dataset by sampling data.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is it really empty?

Copy link
Collaborator

Choose a reason for hiding this comment

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

it will allocate the spaces, so no really empty. But the data are push afterward.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, got it! Maybe then change it to Allocate the space for dataset by sampling data.?

Copy link
Collaborator

Choose a reason for hiding this comment

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

sure, BTW, it will also bucket the feature bins (construct bin_mapper) according to sample data.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated. And one next function description too.

@StrikerRUS StrikerRUS requested a review from guolinke May 26, 2019 16:09
@StrikerRUS StrikerRUS merged commit b1e5a84 into master May 28, 2019
@StrikerRUS StrikerRUS deleted the c_api branch May 28, 2019 09:43
@lock lock bot locked as resolved and limited conversation to collaborators Mar 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants