closes #6556 [feature request] diagnostic for merge.data.table when by = key is not present in dt being merged #6691
Conversation
|
@MichaelChirico can you please approve the workflow only . |
|
FYI GitHub does not automatically recognize 'Closes #XXX' in issue/PR titles, it has to be linked in the description; edited accordingly FWIW, you should also describe the PR in the title in a more self-contained way -- imagine coming back to this PR in 10 years' time. The context of the issue/bug should be long-gone -- rather than having to do a deep-dive to understand the PR, it's better to have a quick blurb giving context & referring to the issue for more context as needed |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6691 +/- ##
=======================================
Coverage 98.61% 98.62%
=======================================
Files 79 79
Lines 14559 14566 +7
=======================================
+ Hits 14358 14365 +7
Misses 201 201 ☔ View full report in Codecov by Sentry. |
spiddy1204
changed the title
|
i chaged it can you guide me a little as lintr is giving error each time i am changing it ,it was giving error in starting also even when i did not introduced any changes in it , can you please suggest what might be the case here . @MichaelChirico |
There was a problem hiding this comment.
It looks like the atime check was expecting to find the issue1 branch in the rdatatable/data.table repository, not your own fork, so it is not a problem with the code you are suggesting to change.
| # Identify which keys are missing from each data table | ||
| missing_x = setdiff(by, nm_x) | ||
| missing_y = setdiff(by, nm_y) | ||
|
|
There was a problem hiding this comment.
@venom1204, do you see a "check warning" from the "lint-r" check around this line when you visit the files changed tab of the pull request? It is asking you to remove the spaces on line 58. Since it doesn't contain any text, it shouldn't contain any spaces either.
|
|
||
| # Construct a more detailed error message | ||
| error_message = "Elements listed in `by` must be valid column names in x and y." | ||
|
|
There was a problem hiding this comment.
Also remove the spaces at the beginning of the line 61.
| error_message = "Elements listed in `by` must be valid column names in x and y." | ||
|
|
||
| if (length(missing_x) > 0) { | ||
| error_message = paste(error_message, "\nMissing columns in 'x':", paste(missing_x, collapse = ", ")) |
There was a problem hiding this comment.
Here, lint-r is asking you to use toString(missing_x) instead of paste(missing_x, collapse = ", ").
| error_message = paste(error_message, "\nMissing columns in 'x':", paste(missing_x, collapse = ", ")) | ||
| } | ||
| if (length(missing_y) > 0) { | ||
| error_message = paste(error_message, "\nMissing columns in 'y':", paste(missing_y, collapse = ", ")) |
| if (length(missing_y) > 0) { | ||
| error_message = paste(error_message, "\nMissing columns in 'y':", paste(missing_y, collapse = ", ")) | ||
| } | ||
|
|
There was a problem hiding this comment.
Again some spaces at the beginning of the line.
| } | ||
| # UPDATED PART ENDS HERE |
There was a problem hiding this comment.
Since you are using a version control system, there is no need to delimit the updated part using comments. The difference between the original code and your change request can be reliably computed by Git itself.
| } | ||
| # UPDATED PART ENDS HERE | ||
|
|
There was a problem hiding this comment.
Again spaces on the otherwise empty line.
| stopf("Elements listed in `by` must be valid column names in x and y") | ||
| by = unname(by) | ||
| by.x = by.y = by | ||
| # UPDATED PART STARTS HERE |
There was a problem hiding this comment.
This comment will stop being helpful after the suggested change is accepted, and we have more reliable ways of finding out which parts of the code are updated.
Closes #6556
which is not helpful as the user would then have to debug which key is not present in which dt. A more informative error would list all keys not present in all dts. For example:
Error in
left_join():! Join columns in
ymust be present in thedata.
✖ Problem with
iso3c.i made some changes to make the error look more informative .