-
Notifications
You must be signed in to change notification settings - Fork 52
Prefer numeric degree RA/Dec fields in VOTable parsing #383
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -225,6 +225,38 @@ export let Catalog = (function () { | |
| // @param raField: index or name of right ascension column (might be undefined) | ||
| // @param decField: index or name of declination column (might be undefined) | ||
| // | ||
| function isUCDField(field, ucdField, oldUcdField) { | ||
| if (!field.ucd) { | ||
| return false; | ||
| } | ||
|
|
||
| var ucd = field.ucd.toLowerCase().trim(); | ||
| return ucd.indexOf(ucdField) == 0 || ucd.indexOf(oldUcdField) == 0; | ||
| } | ||
|
|
||
| function scorePositionField(field) { | ||
| var score = 1; | ||
| var datatype = field.datatype ? field.datatype.toLowerCase() : ""; | ||
| var unit = field.unit ? field.unit.toLowerCase().trim() : ""; | ||
| var isDouble = datatype === "double"; | ||
| var isDeg = unit === "deg"; | ||
|
|
||
| if (datatype && datatype !== "char" && datatype !== "unicodechar") { | ||
| score += 20; | ||
| } | ||
| if (isDouble) { | ||
| score += 20; | ||
| } | ||
| if (isDeg) { | ||
| score += 10; | ||
| } | ||
| if (isDouble && isDeg) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this case ? Having isDouble + isDeg is already better than just isDouble or just isDeg (the two adds up to 30 but having one of them adds up to 10 or 20). |
||
| score += 50; | ||
| } | ||
|
|
||
| return score; | ||
| } | ||
|
|
||
| function findRADecFields(fields, raField, decField) { | ||
| var raFieldIdx, decFieldIdx; | ||
| raFieldIdx = decFieldIdx = null; | ||
|
|
@@ -267,35 +299,29 @@ export let Catalog = (function () { | |
| } | ||
| } | ||
| // if not already given, let's guess position columns on the basis of UCDs | ||
| var guessRaField = raFieldIdx === null; | ||
| var guessDecField = decFieldIdx === null; | ||
| var bestRaScore = -1; | ||
| var bestDecScore = -1; | ||
| for (var l = 0, len = fields.length; l < len; l++) { | ||
| if (raFieldIdx != null && decFieldIdx != null) { | ||
| if (!guessRaField && !guessDecField) { | ||
| break; | ||
| } | ||
|
|
||
| var field = fields[l]; | ||
| if (!raFieldIdx) { | ||
| if (field.ucd) { | ||
| var ucd = field.ucd.toLowerCase().trim(); | ||
| if ( | ||
| ucd.indexOf("pos.eq.ra") == 0 || | ||
| ucd.indexOf("pos_eq_ra") == 0 | ||
| ) { | ||
| raFieldIdx = l; | ||
| continue; | ||
| } | ||
| if (guessRaField && isUCDField(field, "pos.eq.ra", "pos_eq_ra")) { | ||
| var raScore = scorePositionField(field); | ||
| if (raScore > bestRaScore) { | ||
| bestRaScore = raScore; | ||
| raFieldIdx = l; | ||
| } | ||
| } | ||
|
|
||
| if (!decFieldIdx) { | ||
| if (field.ucd) { | ||
| var ucd = field.ucd.toLowerCase().trim(); | ||
| if ( | ||
| ucd.indexOf("pos.eq.dec") == 0 || | ||
| ucd.indexOf("pos_eq_dec") == 0 | ||
| ) { | ||
| decFieldIdx = l; | ||
| continue; | ||
| } | ||
| if (guessDecField && isUCDField(field, "pos.eq.dec", "pos_eq_dec")) { | ||
| var decScore = scorePositionField(field); | ||
| if (decScore > bestDecScore) { | ||
| bestDecScore = decScore; | ||
| decFieldIdx = l; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -131,6 +131,8 @@ export let ProgressiveCat = (function() { | |
|
|
||
| var fields = []; | ||
| var k = 0; | ||
| var bestRaScore = -1; | ||
| var bestDecScore = -1; | ||
| instance.keyRa = instance.keyDec = null; | ||
| xml.querySelectorAll("FIELD").forEach((field) => { | ||
| var f = {}; | ||
|
|
@@ -144,20 +146,18 @@ export let ProgressiveCat = (function() { | |
| if ( ! f.ID) { | ||
| f.ID = "col_" + k; | ||
| } | ||
| if (!instance.keyRa && f.ucd && (f.ucd.indexOf('pos.eq.ra')==0 || f.ucd.indexOf('POS_EQ_RA')==0)) { | ||
| if (f.name) { | ||
| instance.keyRa = f.name; | ||
| } | ||
| else { | ||
| instance.keyRa = f.ID; | ||
| if (isUCDField(f, 'pos.eq.ra', 'pos_eq_ra')) { | ||
| var raScore = scorePositionField(f); | ||
| if (raScore > bestRaScore) { | ||
| bestRaScore = raScore; | ||
| instance.keyRa = f.name || f.ID; | ||
| } | ||
| } | ||
| if (!instance.keyDec && f.ucd && (f.ucd.indexOf('pos.eq.dec')==0 || f.ucd.indexOf('POS_EQ_DEC')==0)) { | ||
| if (f.name) { | ||
| instance.keyDec = f.name; | ||
| } | ||
| else { | ||
| instance.keyDec = f.ID; | ||
| if (isUCDField(f, 'pos.eq.dec', 'pos_eq_dec')) { | ||
| var decScore = scorePositionField(f); | ||
| if (decScore > bestDecScore) { | ||
| bestDecScore = decScore; | ||
| instance.keyDec = f.name || f.ID; | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -168,6 +168,38 @@ export let ProgressiveCat = (function() { | |
| return fields; | ||
| } | ||
|
|
||
| function isUCDField(field, ucdField, oldUcdField) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems to be the same code as in Catalog. Maybe put those definition in Catalog, export them so that they can be imported in ProgressiveCat as well. |
||
| if (!field.ucd) { | ||
| return false; | ||
| } | ||
|
|
||
| var ucd = field.ucd.toLowerCase().trim(); | ||
| return ucd.indexOf(ucdField) == 0 || ucd.indexOf(oldUcdField) == 0; | ||
| } | ||
|
|
||
| function scorePositionField(field) { | ||
| var score = 1; | ||
| var datatype = field.datatype ? field.datatype.toLowerCase() : ""; | ||
| var unit = field.unit ? field.unit.toLowerCase().trim() : ""; | ||
| var isDouble = datatype === "double"; | ||
| var isDeg = unit === "deg"; | ||
|
|
||
| if (datatype && datatype !== "char" && datatype !== "unicodechar") { | ||
| score += 20; | ||
| } | ||
| if (isDouble) { | ||
| score += 20; | ||
| } | ||
| if (isDeg) { | ||
| score += 10; | ||
| } | ||
| if (isDouble && isDeg) { | ||
| score += 50; | ||
| } | ||
|
|
||
| return score; | ||
| } | ||
|
|
||
| function getSources(instance, csv, fields) { | ||
| // TODO : find ra and dec key names (see in Catalog) | ||
| if (!instance.keyRa || ! instance.keyDec) { | ||
|
|
||
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 tests seems to prefer non char. the idea is to prefer numeric values I think (int, double, float, complex, boolean, short). Do we consider integer/complex/boolean to be legit for a position so that we increase the score comparatively to column position that would be in sexagesimal (char) ? I would just add a bonus for double/float.