-
Notifications
You must be signed in to change notification settings - Fork 24
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
GAIA DR3 radial velocities ingest #253
Conversation
@@ -6852,13 +6852,13 @@ | |||
"description": "ISOCAM observations of the Chamaeleon I dark cloud" | |||
}, | |||
{ | |||
"publication": "Lópe04", | |||
"publication": "L\u00f3pe04", |
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.
We've got some unicode in here and I'm not sure what to do. @dr-rodriguez , thoughts? I feel like we've dealt with this before but I don't remember the discussion.
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 should be fixed now, but we will need to merge the latest changes from main into this branch and re-ingest the RVs
"bibcode": "2004A&A...416..555L", | ||
"doi": "10.1051/0004-6361:20031720", | ||
"description": "The brown dwarf population in the Chamaeleon I cloud" | ||
}, | ||
{ | ||
"publication": "Góme01", | ||
"publication": "G\u00f3me01", |
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.
more unicode. Not sure if we want \u00f3 or ó. @dr-rodriguez ?
@@ -14,7 +14,7 @@ | |||
], | |||
"Names": [ | |||
{ | |||
"other_name": "2MASS J16060391–2056443" | |||
"other_name": "2MASS J16060391\u20132056443" |
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.
more unicode. \u2013 instead of -
@@ -1,7 +1,7 @@ | |||
{ | |||
"Sources": [ | |||
{ | |||
"source": "2MASS J16060391–2056443", | |||
"source": "2MASS J16060391\u20132056443", |
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.
unicode.
"radial_velocity": 11.02063274383545, | ||
"radial_velocity_error": 37.235923767089844, |
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.
Do we want to ingest RVs that have a very large uncertainty? @Will-Cooper , thoughts?
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.
That's a bigger discussion @kelle; I wouldn't trust any RVs for UCDs from Gaia to do any science but is the idea of the database to record all forms of data or just what we are saying is trustworthy? I see the Gaia G mag is 13.7 and the limit for RV processing is 14, so it's borderline snuck in there at the most uncertain bound.
# gaia_dr3_data.write(dr3_data_file_string, format='votable') | ||
gaia_dr3_data = Table.read(dr3_data_file_string, format='votable') | ||
|
||
# ingest_sources(db, gaia_dr3_data['designation'], 'GaiaDR3') |
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.
Did you run ingest_sources
? I think ingest_sources
is the script which is giving us the unicode characters and the one which needs to be modified so that this won't happen in the future.
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.
@kelle I think I did, but I think @dr-rodriguez said to mention when something like this happens so that he can fix it or I think he mentioned adding these special characters so that this doesn't happen
Co-authored-by: Kelle Cruz <[email protected]>
To address unicode issue, try using the normalize function in the I don't have your XML file so I cant test it. [edited to use NFKC instead of NFKD] |
@kelle I thought this ingest was done, is there a reason that it was never merged? I am assuming it may have had something to do with the JSON files, so it was labeled as low priority. |
I will try to finish this up. |
Closes #249