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

Column naming for empty string and duplicate NA label #6795

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Divendra2006
Copy link

Fixes #5605

This PR addresses the issues of Handling empty strings with "empty_string" and ensuring unique column.

Handling Empty Strings in Column Names

Added a function handle_empty_strings to replace empty strings in column names with "empty_string".

Ensuring Unique Column Names

Added a function ensure_unique_names to ensure column names are unique by appending a suffix if duplicates are found.

Improved fill.default Handling

Explicitly handled fill.default when fun.aggregate is used and fill is NULL.
When fun.aggregate is used and fill is NULL, missing values in the reshaped data need to be filled with a default value.This change ensures that fill.default is computed correctly and used to fill missing values, improving consistency and preventing errors.

Added a check to ensure the length of names matches the length of the vector before setting the names attribute.

if there is any improvement needed in the code than tell me.

@Divendra2006 Divendra2006 requested a review from tdhock as a code owner February 3, 2025 21:08
Copy link

codecov bot commented Feb 3, 2025

Codecov Report

Attention: Patch coverage is 84.21053% with 3 lines in your changes missing coverage. Please review.

Project coverage is 98.58%. Comparing base (f9c2824) to head (6a4fe65).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
R/fcast.R 84.21% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6795      +/-   ##
==========================================
- Coverage   98.60%   98.58%   -0.02%     
==========================================
  Files          79       79              
  Lines       14642    14657      +15     
==========================================
+ Hits        14438    14450      +12     
- Misses        204      207       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@tdhock tdhock left a comment

Choose a reason for hiding this comment

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

before asking for review, please click "Files changed" tab and make sure that only a minimal set of changes that is relevant to the PR appears.

here there are many irrelevant changes should be reverted before review (adding empty lines, and removing comments)

R/fcast.R Outdated
@@ -147,18 +162,21 @@ dcast.data.table = function(data, formula, fun.aggregate = NULL, sep = "_", ...,
if (is.function(dat[[i]]))
stopf("Column [%s] not found or of unknown type.", deparse(x))
}

Copy link
Member

Choose a reason for hiding this comment

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

please undo addition of empty lines

R/fcast.R Outdated
@@ -168,9 +186,10 @@ dcast.data.table = function(data, formula, fun.aggregate = NULL, sep = "_", ...,
subset = m[["subset"]][[2L]]
if (!is.null(subset)) {
if (is.name(subset)) subset = as.call(list(quote(`(`), subset))
idx = which(eval(subset, data, parent.frame())) # any advantage thro' secondary keys?
Copy link
Member

Choose a reason for hiding this comment

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

please undo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dcast outputs column name V1 for empty string
2 participants