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] Make signature incorrect in docs for JSON TableReader #35167

Closed
zfoobar opened this issue Apr 16, 2023 · 4 comments · Fixed by #37301
Closed

[Docs] Make signature incorrect in docs for JSON TableReader #35167

zfoobar opened this issue Apr 16, 2023 · 4 comments · Fixed by #37301

Comments

@zfoobar
Copy link

zfoobar commented Apr 16, 2023

Describe the bug, including details regarding any error messages, version, and platform.

The argument count appears wrong in docs for for the JSON table reader method..

docs:

st = arrow::json::TableReader::Make(pool, input, read_options,
parse_options, &reader);

object signature in json/reader.cc:

Result<std::shared_ptr<TableReader>> TableReader::Make(
MemoryPool* pool, std::shared_ptr<io::InputStream> input,
 const ReadOptions& read_options, const ParseOptions& parse_options)

It looks correct in the actual API docs.

Component(s)

C++

@zfoobar zfoobar changed the title [Documentation] method signature incorrect in docs for JSON TableReader [Documentation] object signature incorrect in docs for JSON TableReader Apr 16, 2023
@zfoobar zfoobar changed the title [Documentation] object signature incorrect in docs for JSON TableReader [Docs] Make signature incorrect in docs for JSON TableReader Apr 16, 2023
@Tej-ashwani
Copy link

Result<std::shared_ptr> TableReader::Make(
MemoryPool* pool, std::shared_ptrio::InputStream input,
const ReadOptions& read_options, const ParseOptions& parse_options)

Based on the information provided, the bug seems to be related to the argument count discrepancy in the documentation for the TableReader::Make method in the json/reader.cc file.

However, the bug report suggests that the argument count appears wrong in the documentation for this method.

It is important to note that without further details, such as the specific error messages, version of the software/library, and platform where the bug is encountered, it is difficult to provide a more specific analysis or solution. Additionally, if there is an inconsistency between the documentation and the actual API, it might be necessary to refer to the API documentation or consult the library's developers for clarification.

Send a message.

@westonpace
Copy link
Member

Yes, you are correct. There was a change a while back from a status-returning method with an out-parameter to a result-returning method.

The example in the docs will need to be updated.

Would you be interested in submitting a PR?

@rsm-23
Copy link
Contributor

rsm-23 commented Aug 22, 2023

take

@rsm-23
Copy link
Contributor

rsm-23 commented Aug 22, 2023

@westonpace I have created a PR for the above. Can you please review it?

kou pushed a commit that referenced this issue Aug 23, 2023
### Rationale for this change

The current document causes a build error.

### What changes are included in this PR?

Use the current API.

### Are these changes tested?

No.

### Are there any user-facing changes?

Yes.

* Closes: #35167

Lead-authored-by: Rajat Subhra Mukherjee <raromukherjee@gmail.com>
Co-authored-by: Will Jones <willjones127@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
@kou kou added this to the 14.0.0 milestone Aug 23, 2023
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
…pache#37301)

### Rationale for this change

The current document causes a build error.

### What changes are included in this PR?

Use the current API.

### Are these changes tested?

No.

### Are there any user-facing changes?

Yes.

* Closes: apache#35167

Lead-authored-by: Rajat Subhra Mukherjee <raromukherjee@gmail.com>
Co-authored-by: Will Jones <willjones127@gmail.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants