-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Simplified and Extended "Updating by Reference" Section in Joins Vignette #6847
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6847 +/- ##
=======================================
Coverage 98.59% 98.59%
=======================================
Files 79 79
Lines 14661 14665 +4
=======================================
+ Hits 14455 14459 +4
Misses 206 206 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
I think it is best to ping author of this vignette. I haven't had yet opportunity to read the original version. |
vignettes/datatable-joins.Rmd
Outdated
- The on = .(id = product_id) ensures that updates happen based on matching IDs. | ||
- This method modifies Products in place, avoiding unnecessary copies. | ||
|
||
2) If we need to get the latest price and date (instead of all matches), we can still use := efficiently: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure we've cleanly explained the difference between this join and the previous one. The only difference I see is tail()
vs. last()
, am I missing something?
IIUC the difference between tail()
and last()
would be that last()
can skip NA
values, right?
vignettes/datatable-joins.Rmd
Outdated
- We add a new `last_updated` column to track when the price was last changed. | ||
- The `by = .EACHI` ensures that the `tail` function is applied for each product in `ProductPriceHistory`. | ||
#### Let's update our `Products` table with the latest price from `ProductPriceHistory`: | ||
```{r Simple One-to-One Update} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are these valid {knitr} chunk names? regardless, please use machine-readable names (a la https://style.tidyverse.org/files.html)
@lucasmation @MichaelChirico thanks. |
@venom1204, what is the update on this, have you tested the code .
|
@MichaelChirico I’ve minimized the changes I introduced while ensuring they contain sufficient information and remain easy to understand. Could you review them and let me know if any further simplification is needed, or if they are good as they are? |
@iagogv3 I’ve incorporated your suggestions into my latest version. Could you review the updates and let me know if any further improvements are needed? Your feedback would be greatly appreciated! |
Simplified and extended the Updating by Reference section in the Joins in data.table vignette.
closes #6846
in this i
@jangorecki can you please review this when you have time and suggest me if any changes are needed
thank you