wkiri / MTE

Mars Target Encyclopedia
Apache License 2.0
5 stars 0 forks source link

Issue40 mer b #49

Open wkiri opened 1 year ago

wkiri commented 1 year ago

This PR covers updates needed to generate the MER-B content for the MTE PDS bundle, including:

@stevenlujpl Could you take a look at the updates to the scripts? (ingest_sqlite.py, update_sqlite.py, and generate_pds4_bundle.py) No need to look at the template and alias files unless you want to. However, I'd appreciate your check over the three Python scripts mentioned above, to see if there are any issues you spot.

wkiri commented 10 months ago

@stevenlujpl Steven, I just noticed that this PR is still open. Would you have a chance to look at the changes to these three scripts? If too much time has passed or this would be too onerous, just let me know. :)

stevenlujpl commented 10 months ago

Hi @wkiri , looking at generate_pds4_bundle.py and the MERB template files, I think they are fine for generating MERB bundles. However, I can't remember why we also need to modify update_sqlite.py and ingest_sqlite.py. It seems these two scripts don't have mission-specific code, which means if they work for other missions, they should also be working fine for MERB.

wkiri commented 10 months ago

@stevenlujpl Great point! I refreshed my memory of the changes to these files. For ingest_sqlite.py there is one changed comment and a change to restrict to types Element/Mineral instead of anything not Target: https://github.com/wkiri/MTE/pull/49/files#diff-6e98e36ee87c6aeae75ae646cf7b013ba17b3834409f0b8ee138c1208627a2d4

For update_sqlite.py, there are changes in how the aliases.csv file is handled, to use the csv library instead of depending on a comma-based split, because with MER-B we found that a Target name can contain a comma (Berry_Bowl,_Empty): https://github.com/wkiri/MTE/pull/49/files#diff-e60cbd2ab40852a237954c25352784060f4ea5d54caef2c685dca80977e93b83

stevenlujpl commented 10 months ago

Hi @wkiri , thanks for posting the links to the changes.

For update_sqlite.py, I think using the csv library is a more robust way of handling csv files.

For ingest_sqlite.py, it seems the change only makes sense if we added another type to r['label']. In addition to Target, Element, and Mineral, do you remember if we added another type to r['label']? Anyway, I think the current implementation is more robust because we only want the Element and Mineral entities to be ingested.