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

[BI-2055] - Remove missingValueString tablesaw values #1

Merged
merged 3 commits into from
Jan 15, 2025
Merged

[BI-2055] - Remove missingValueString tablesaw values #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4d567b8
[BI-2055] - Remove missingValueString tablesaw values
HMS17 Dec 3, 2024
03df29c
[BI-2055] - Replace null in test files with empty string to stop colu…
HMS17 Dec 5, 2024
2986e5d
[BI-2055] - Code review comment fix
HMS17 Jan 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 1 addition & 9 deletions core/src/main/java/tech/tablesaw/io/TypeUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,10 @@
@Immutable
public final class TypeUtils {

/** Strings representing missing values in, for example, a CSV file that is being imported */
private static final String missingInd1 = "NaN";

private static final String missingInd2 = "*";
private static final String missingInd3 = "NA";
private static final String missingInd4 = "null";
private static final String missingInd5 = "N/A";

// No default missing indicators
// TODO: Allow this to be configurable?
public static final ImmutableList<String> MISSING_INDICATORS =
ImmutableList.of(missingInd1, missingInd2, missingInd4, missingInd5);
ImmutableList.of();
Comment on lines 25 to +28
Copy link

Choose a reason for hiding this comment

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

Does it make sense to keep lines 25-31 around in this file at all, if those variables are not used?

Copy link
Author

Choose a reason for hiding this comment

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

Good catch! I was admittedly doing the bare minimum to obtain the desired change in functionality lest I accidentally break something else, but that is something that can be reasonably removed, so just added a commit!


/** Private constructor to prevent instantiation */
private TypeUtils() {}
Expand Down
2 changes: 1 addition & 1 deletion core/src/test/java/tech/tablesaw/io/csv/CsvReaderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ void testWithMissingValue2() throws IOException {

Table t = Table.read().csv("../data/missing_values2.csv");
assertEquals(1, t.stringColumn(0).countMissing());
assertEquals(1, t.numberColumn(1).countMissing());
assertEquals(0, t.numberColumn(1).countMissing());
Copy link

Choose a reason for hiding this comment

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

Why was this change needed?

Copy link
Author

Choose a reason for hiding this comment

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

The test counts missing values, which includes both values in the MISSING_INDICATORS that were turned to an empty string, as well as the empty string itself. Removing values from MISSING_INDICATORS meant that one of the values in the csv was no longer a missing value, and so the count in the test had to be adjusted.

assertEquals(0, t.numberColumn(2).countMissing());
}

Expand Down
Loading
Loading