s434730/RandomSec
3
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Remove shortest-column-name ordering

Browse Source 
Refs #2863
The tree importer sorts columns/column groups by how populated
they are, which is of arguable utility, but the tie-breaker
of ordering by shortest column name is completely silly.

This change removes that and, in conjunction with a stable sort
algorithm, will preserve the original order of the columns.
This commit is contained in:
Tom Morris 2020-07-02 15:42:55 -04:00
parent 28a9f68236
commit d3db73aa67
2 changed files with 11 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
main/src/com/google/refine/importers/tree/TreeImportUtilities.java
View File

@ -73,8 +73,7 @@ public abstract class TreeImportUtilities {
return o1.blankOnFirstRow ? 1 : -1; return o1.blankOnFirstRow ? 1 : -1;
} }
int c = o2.nonBlankCount - o1.nonBlankCount; return o2.nonBlankCount - o1.nonBlankCount;
return c != 0 ? c : (o1.name.length() - o2.name.length());
} }
}); });
@ -85,6 +84,7 @@ public abstract class TreeImportUtilities {
project.columnModel.columns.add(column); project.columnModel.columns.add(column);
} }
// The LinkedHashMap iterator will guaranteed that the list is arranged in order found
List<ImportColumnGroup> subgroups = new ArrayList<ImportColumnGroup>(columnGroup.subgroups.values()); List<ImportColumnGroup> subgroups = new ArrayList<ImportColumnGroup>(columnGroup.subgroups.values());
Collections.sort(subgroups, new Comparator<ImportColumnGroup>() { Collections.sort(subgroups, new Comparator<ImportColumnGroup>() {
@Override @Override
@ -96,9 +96,8 @@ public abstract class TreeImportUtilities {
// TODO: From a human factors point of view, we probably want // TODO: From a human factors point of view, we probably want
// to try to preserve the order that we found things in the XML // to try to preserve the order that we found things in the XML
// Sort by most populated first, then shortest name // Sort by most populated first, but leave order unchanged if they're equal
int c = o2.nonBlankCount - o1.nonBlankCount; return o2.nonBlankCount - o1.nonBlankCount;
return c != 0 ? c : (o1.name.length() - o2.name.length());
} }
}); });

12
main/tests/server/src/com/google/refine/importers/XmlImporterTests.java
View File

@ -51,6 +51,7 @@ import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.refine.importers.XmlImporter; import com.google.refine.importers.XmlImporter;
import com.google.refine.importers.tree.TreeImportingParserBase; import com.google.refine.importers.tree.TreeImportingParserBase;
import com.google.refine.importing.ImportingJob; import com.google.refine.importing.ImportingJob;
import com.google.refine.model.ColumnGroup;
import com.google.refine.model.Row; import com.google.refine.model.Row;
import com.google.refine.util.JSONUtilities; import com.google.refine.util.JSONUtilities;
import com.google.refine.util.ParsingUtilities; import com.google.refine.util.ParsingUtilities;
@ -197,11 +198,12 @@ public class XmlImporterTests extends ImporterTest {
assertProjectCreated(project, 5, 6); assertProjectCreated(project, 5, 6);
Assert.assertEquals(project.columnModel.columnGroups.size(),1); Assert.assertEquals(project.columnModel.columnGroups.size(),1);
Assert.assertEquals(project.columnModel.columnGroups.get(0).keyColumnIndex, 2); ColumnGroup cg0 = project.columnModel.columnGroups.get(0);
Assert.assertEquals(project.columnModel.columnGroups.get(0).startColumnIndex, 2); Assert.assertEquals(cg0.keyColumnIndex, 1);
Assert.assertNull(project.columnModel.columnGroups.get(0).parentGroup); Assert.assertEquals(cg0.startColumnIndex, 1);
Assert.assertEquals(project.columnModel.columnGroups.get(0).subgroups.size(),0); Assert.assertNull(cg0.parentGroup);
Assert.assertEquals(project.columnModel.columnGroups.get(0).columnSpan,2); Assert.assertEquals(cg0.subgroups.size(),0);
Assert.assertEquals(cg0.columnSpan,2);
} }
//------------helper methods--------------- //------------helper methods---------------