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

GH-38216: [R] open_dataset(format = "json") not documented #38258

Conversation

Divyansh200102
Copy link
Contributor

@Divyansh200102 Divyansh200102 commented Oct 13, 2023

@github-actions
Copy link

Thanks for opening a pull request!

If this is not a minor PR. Could you open an issue for this pull request on GitHub? https://github.com/apache/arrow/issues/new/choose

Opening GitHub issues ahead of time contributes to the Openness of the Apache Arrow project.

Then could you also rename the pull request title in the following format?

GH-${GITHUB_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

or

MINOR: [${COMPONENT}] ${SUMMARY}

In the case of PARQUET issues on JIRA the title also supports:

PARQUET-${JIRA_ISSUE_ID}: [${COMPONENT}] ${SUMMARY}

See also:

@Divyansh200102 Divyansh200102 changed the title Fix [R] open_dataset(format = "json") not documented Fixes [R] open_dataset(format = "json") not documented Oct 13, 2023
@thisisnic thisisnic changed the title Fixes [R] open_dataset(format = "json") not documented GH-38216: [R] open_dataset(format = "json") not documented Oct 13, 2023
@github-actions
Copy link

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

Copy link
Member

@thisisnic thisisnic 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 making this PR, @Divyansh200102! A couple of things to suggest:

  1. The ".Rd" files in the package are automatically generated from the Roxygen headers above the functions in the package, and so the change needs to be made by adding the documentation in line 115 in dataset.R:

arrow/r/R/dataset.R

Lines 114 to 115 in 4bf777a

#' * "tsv", equivalent to passing `format = "text", delimiter = "\t"`
#'

Then, if you run devtools::document() in R the change in open_dataset.Rd will be automatically generated. (if you're only making the changes in the GitHub editor, let me know and I can run devtools::document() myself and push the change for you)

  1. We only currently support newline-delimited JSON (aka ND-JSON), so we should be clear about that so that users don't mistakenly think we support all JSON

@github-actions github-actions bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting review Awaiting review awaiting changes Awaiting changes labels Oct 13, 2023
@Divyansh200102 Divyansh200102 force-pushed the open_dataset(format-=-json-)-not-documented branch from b2ecd98 to 1ff9336 Compare October 15, 2023 11:38
@Divyansh200102
Copy link
Contributor Author

Divyansh200102 commented Oct 15, 2023

is it fine now? and for some reason i am not able to setup the r so can you run devtools::document() in R and push the changes in open_dataset.Rd in yourself

@thisisnic
Copy link
Member

Great, the comment is in the right file! Just needs updating to refer to newline-delimited JSON rather than just JSON.

@jonkeane
Copy link
Member

@github-actions autotune

@jonkeane
Copy link
Member

I tried to fix the docs on using out bot, but looks like the github bot can’t push to this fork: https://github.com/apache/arrow/actions/runs/6550133348/job/17788582932#step:13:8

I'm not sure if this is something you've configured @Divyansh200102 to disallow pushes to your fork from upstream, or if it's something that is wrong with the bot generally.

@Divyansh200102
Copy link
Contributor Author

I tried to fix the docs on using out bot, but looks like the github bot can’t push to this fork: https://github.com/apache/arrow/actions/runs/6550133348/job/17788582932#step:13:8

I'm not sure if this is something you've configured @Divyansh200102 to disallow pushes to your fork from upstream, or if it's something that is wrong with the bot generally.

shall i create a new pull request??

@jonkeane
Copy link
Member

shall i create a new pull request??

Nope, don't worry about it — I pulled it locally and updated them. Once the CI is green I'll approve + merge.

Thanks for your contribution!

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting change review Awaiting change review labels Oct 17, 2023
@jonkeane jonkeane merged commit ac581fd into apache:main Oct 17, 2023
11 checks passed
@jonkeane jonkeane removed the awaiting merge Awaiting merge label Oct 17, 2023
@Divyansh200102 Divyansh200102 deleted the open_dataset(format-=-json-)-not-documented branch October 18, 2023 03:55
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit ac581fd.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

JerAguilon pushed a commit to JerAguilon/arrow that referenced this pull request Oct 23, 2023
…che#38258)

fixes apache#38216 
* Closes: apache#38216

Lead-authored-by: Divyansh200102 <[email protected]>
Co-authored-by: Divyansh200102 <[email protected]>
Co-authored-by: Jonathan Keane <[email protected]>
Signed-off-by: Jonathan Keane <[email protected]>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…che#38258)

fixes apache#38216 
* Closes: apache#38216

Lead-authored-by: Divyansh200102 <[email protected]>
Co-authored-by: Divyansh200102 <[email protected]>
Co-authored-by: Jonathan Keane <[email protected]>
Signed-off-by: Jonathan Keane <[email protected]>
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…che#38258)

fixes apache#38216 
* Closes: apache#38216

Lead-authored-by: Divyansh200102 <[email protected]>
Co-authored-by: Divyansh200102 <[email protected]>
Co-authored-by: Jonathan Keane <[email protected]>
Signed-off-by: Jonathan Keane <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[R] open_dataset(format = "json") not documented
3 participants