Skip to content

Conversation

hiroyuki-sato
Copy link
Collaborator

@hiroyuki-sato hiroyuki-sato commented Jun 16, 2025

Rationale for this change

This is the sub issue #46808

Since EXTRACT_ALL is set YES in Doxygen, I expect that methods without comments will also be included in the API Doc, but some methods are not documented.

For example FixedSizeListType::list_size

What changes are included in this PR?

Add missing method description in the type.h file

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

Copy link

⚠️ GitHub issue #46818 has been automatically assigned in GitHub to PR creator.

@hiroyuki-sato
Copy link
Collaborator Author

@github-actions crossbow submit preview-docs

Copy link

Revision: f0e9bf8

Submitted crossbow builds: ursacomputing/crossbow @ actions-94ac5ab264

Task Status
preview-docs GitHub Actions

@hiroyuki-sato
Copy link
Collaborator Author

I wrote method description as a draft. Please feel free comments(modify) if you feel this PR useful.

http://crossbow.voltrondata.com/pr_docs/46819/cpp/api/datatype.html#classarrow_1_1_fixed_size_list_type

@hiroyuki-sato
Copy link
Collaborator Author

@raulcd @pitrou Could you review for this? Or could you request review someone who proper maintainer.
(They both had reviewed #46540 before, so I consulted with them.)

Copy link
Member

@pitrou pitrou left a comment

Choose a reason for hiding this comment

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

Thanks for taking a look at this @hiroyuki-sato ! I posted a few comments, but the general theme is that you should take some time writing some docstrings that are actually useful and informative.

@@ -1337,20 +1339,25 @@ class ARROW_EXPORT MapType : public ListType {

explicit MapType(std::shared_ptr<Field> value_field, bool keys_sorted = false);

// Validating constructor
/// Validating constructor
Copy link
Member

Choose a reason for hiding this comment

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

Please let's make this a real informative docstring too.

@pitrou
Copy link
Member

pitrou commented Jun 23, 2025

@zanmato1984 In case you want to chime in.

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jun 23, 2025
@hiroyuki-sato
Copy link
Collaborator Author

@pitrou Thanks. I'll try to write useful description. Please review later agin.

hiroyuki-sato and others added 2 commits June 23, 2025 22:28
Co-authored-by: Antoine Pitrou <[email protected]>
Co-authored-by: Antoine Pitrou <[email protected]>
@hiroyuki-sato hiroyuki-sato marked this pull request as draft June 24, 2025 13:23
@hiroyuki-sato
Copy link
Collaborator Author

@github-actions crossbow submit preview-docs

Copy link

Revision: 6604ac8

Submitted crossbow builds: ursacomputing/crossbow @ actions-8a892a3aeb

Task Status
preview-docs GitHub Actions

@hiroyuki-sato
Copy link
Collaborator Author

I added MapType documentation.

http://crossbow.voltrondata.com/pr_docs/46819/cpp/api/datatype.html#classarrow_1_1_map_type

I will continue working.

It will take a time because I don't know much the API usage. (That's why I needed the document 😄 )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants