-
Notifications
You must be signed in to change notification settings - Fork 68
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
Classifier #1357
Classifier #1357
Conversation
cdm/core/src/test/java/ucar/nc2/ncml/TestEnhanceClassifier.java
Outdated
Show resolved
Hide resolved
@@ -46,25 +74,64 @@ public int[] classifyDoubleArray(Array arr) { | |||
return classifiedArray; | |||
} | |||
|
|||
|
|||
|
|||
/** for a single double */ | |||
public int classifyArray(double val) { | |||
if (val >= 0) { |
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.
Is this classifyArray
still being used? maybe we can delete this function as it was the placeholder version and add the categories to the ncml in testClassifier.ncml
so your original tests keep working. It's possible that this is causing some of the test issues as the original tests may be using this code path and you only added the tolerance to the comparison below in classifyArrayAttribute
return classifiedVal; | ||
} | ||
|
||
public int classifyArrayAttribute(double val) { | ||
for (int[] rule : rules) { | ||
if (val > rule[0] && val <= rule[1] + ucar.nc2.util.Misc.defaultMaxRelativeDiffFloat) { |
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.
minor style comment: I would add an import statement for
import ucar.nc2.util.Misc;
so that this can just be Misc.defaultMaxRelativeDiffFloat
@@ -78,6 +83,7 @@ public void testEnhanceClassifier_floats() throws IOException { | |||
assertThat(floatMix.getDataType()).isEqualTo(DataType.FLOAT); | |||
assertThat(floatMix.attributes().hasAttribute("classify")).isTrue(); | |||
Array datafloatsMix = floatMix.read(); | |||
// assertThat(datafloatsMix).isEqualTo(DATA_mixNumbers); |
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.
this can probably go?
// assertThat(datafloatsMix).isEqualTo(DATA_mixNumbers); |
// List<Attribute> Whatever = Classify_Specsx.getAttributes(); | ||
// Classifier classifier = new Classifier(Whatever); |
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.
this can probably go?
// List<Attribute> Whatever = Classify_Specsx.getAttributes(); | |
// Classifier classifier = new Classifier(Whatever); |
|
||
try (NetcdfFile ncfile = NetcdfDatasets.openDataset(dataDir + "testAddToClassifier.ncml", true, null)) { | ||
|
||
Variable Classify_Specsx = ncfile.findVariable("class_specs"); |
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.
Minor style issue: would use lower camel case for the variable name, something like:
Variable Classify_Specsx = ncfile.findVariable("class_specs"); | |
Variable classifySpecs = ncfile.findVariable("class_specs"); |
finalize reviews
9d1b75a
to
5ce0491
Compare
Description of Changes
Using ncml attributes description, the classifier can now modify the array into any number of different classes with different ranges!
example:
will sort numbers from -inf to 0 into category 0, and numbers from 100 to +inf into category 1000.
PR Checklist
until ready for review