SIMPLE-AstroDB / SIMPLE-db

BSD 3-Clause "New" or "Revised" License
11 stars 21 forks source link

First ingest wise_1810-1010 #427

Closed LishaRamon closed 8 months ago

LishaRamon commented 9 months ago

Data of Wise_1810-1010 that are ingested in this script are publications, sources, names and parallaxes.

Link to relevant issue: Closes #258

For data ingests:

LishaRamon commented 9 months ago

Will be adding tests next.

kelle commented 9 months ago

This looks good but somehow you're also modifying 2mass_j12560183-1257276.json. That should not be part of this pull request.

LishaRamon commented 9 months ago

A .json file wasnt produced when saving the db, assuming because the code doesn't ingest yet. There is an error with source not being recognized as well.

kelle commented 9 months ago

Good work! Other than the one bug with source, this looks great!

LishaRamon commented 9 months ago

Trying to push current code, but saying I need to do a pull request first. Approved the changes you sent and implemented them. Used git fetch to make sure the files are the same. Not able to push at the moment, any suggestions?

LishaRamon commented 9 months ago

I believe all of them pushed (showed up together). Most recent one (push attempt) should be the current one. One issue is there being a similar publication in the data. And occasionally says "add_publications" is not defined.

LishaRamon commented 9 months ago

Both commits have the same script, just got pushed twice on accident.

kelle commented 9 months ago

The parameter names MUST be in the Parameters table. That's why you're getting foreign key constraint errors. "Radius" is not in the Parameters table.

kelle commented 9 months ago

Good work!

LishaRamon commented 9 months ago

The json file you linked is "2MASS J12560215-1257217". But the file you wanted me to edit was "2MASS J12560183-1257276". Just checking if that's what you meant to send.

Effective temperature would be added as another parameter is that right? Since it's from the poster, what would the reference change to? Since we didnt ingest the poster

kelle commented 9 months ago

Yes, you're right. I linked to the wrong file.

kelle commented 9 months ago

And yes, effective temperature is another parameter.

LishaRamon commented 9 months ago

Since the value is less than or equal to 1000, would there be an appropriate error value to input? How would we reference the poster in the db?

Corrected the right json file, appreciate the link, it was right above it :)

kelle commented 9 months ago

Look at Table 6 from the paper that's linked in the issue. Do not use the poster values.

LishaRamon commented 9 months ago

The commit named "Jsons updated / t eff edit needed" with the number '[b6e5a08]" is the correct one with current changes. Others were repeated trial push attempts with the previous error. Just to make 4 file updates easier to see:

Versions.py- latest version was added/updated 2MASS J12560183-1257276.json- checked it was identical to the original db CWISEP J181006.00-101001.1.json- under modeled parameters: mass(value_error changed to null and comments added/corrected), radius(unit changed to R_sun) ingest_wise_1810-1010.py- Effective Temp parameters were added, checked, still does not show in db/json file

kelle commented 9 months ago

Good job getting the push to work!

I pushed a commit which removed the 1256 json file so you'll need to pull again.

did you save the database after adding the T_eff?

kelle commented 9 months ago

T eff test looks great! Go ahead and add tests for the other parameters.