Skip to content

Conversation

@wkiri
Copy link
Owner

@wkiri wkiri commented Oct 1, 2022

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

  • Addition of MER-B aliases file
  • Updates to template files
  • Updates to some core MTE scripts

@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 wkiri requested a review from stevenlujpl October 1, 2022 01:30
@wkiri
Copy link
Owner Author

wkiri commented Aug 14, 2023

@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
Copy link
Collaborator

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
Copy link
Owner Author

wkiri commented Aug 18, 2023

@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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants