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.

Sign up for GitHub

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

Jump to bottom

Performance and optimization vignette #277

Merged
merged 5 commits into from
Sep 19, 2024
Merged

Performance and optimization vignette #277

merged 5 commits into from
Sep 19, 2024

Conversation

yjunechoe
Copy link
Collaborator

@trangdata let's work on this branch for #274!

@trangdata
Copy link
Collaborator

@yjunechoe I added the oa_generate() section and everything looks good! (Sorry for the sentence breaks... I used Visual mode — should have done it in two commits so it's easier for you to see what I changed.)

@trangdata
Copy link
Collaborator

trangdata commented Sep 19, 2024 •
edited
Loading

Hmm oa_ngram test is failing because the endpoint is currently not in service:

The n-gram API endpoint is not currently in service. The n-grams are still used on our backend to help power fulltext search. If you have any questions about this, please submit a support ticket.

yjunechoe
yjunechoe commented Sep 19, 2024
View reviewed changes
Copy link
Collaborator Author

@yjunechoe yjunechoe left a comment

Choose a reason for hiding this comment

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

Just leaving 1 thought - up to you to see if it fits!

vignettes/articles/performance-optimization.Rmd Show resolved Hide resolved
@yjunechoe
Copy link
Collaborator Author

yjunechoe commented Sep 19, 2024 •
edited
Loading

@trangdata A round of small edits from me too! I made a small tweak to the unevaluated oa_generate() chunk to have rds files save to tempdir, so people can just copy and run the code to test it without it polluting their working dir.

Good to go from my end!

@trangdata trangdata merged commit 1ed6887 into ropensci:main Sep 19, 2024
7 checks passed
@trangdata trangdata mentioned this pull request Sep 19, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trangdata trangdata trangdata approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yjunechoe @trangdata