Add pdsfile support for the f ring and b ring bundles#89
Open
Add pdsfile support for the f ring and b ring bundles#89
Conversation
absolute path of a filespec.
Collaborator
|
I talked this over with Mia and Matt. Here's what we'd like to do for the archives:
Each of the last 3 will include:
|
…_mosaics_rsfrench2025
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
pdsfile/pds4file/rules/cassini_iss_fring_mosaics_rsfrench2025.py (1)
90-96:⚠️ Potential issue | 🟠 MajorRoute document associations to in-bundle
document/paths.Line 93 and Line 94 emit
documents/...patterns, while this module’s products and archives usebundles/.../document/.... This can breakassociated_abspaths('documents')resolution for these products.Suggested fix
associations_to_documents = translator.TranslatorByRegex([ - (r'bundles/cassini_iss_fring_mosaics_rsfrench2025[^/]*', 0, + (r'bundles/(cassini_iss_fring_mosaics_rsfrench2025/cassini_iss_fring_mosaics_rsfrench2025[^/]*)(|/.*)$', 0, [ - r'documents/cassini_iss_fring_mosaics_rsfrench2025[^/]*', - r'documents/cassini_iss_fring_mosaics_rsfrench2025[^/]*/.*', + r'bundles/\1/readme.txt', + r'bundles/\1/document/user_guide/f-ring-mosaics-user-guide.lblx', + r'bundles/\1/document/user_guide/f-ring-mosaics-user-guide.pdf', + r'bundles/\1/document/supplemental/global_mosaic_index.lblx', + r'bundles/\1/document/supplemental/global_mosaic_index.tab', + r'bundles/\1/document/supplemental/global_mosaic_bkg_sub_index.lblx', + r'bundles/\1/document/supplemental/global_mosaic_bkg_sub_index.tab', + r'bundles/\1/document/supplemental/global_reproj_img_index.lblx', + r'bundles/\1/document/supplemental/global_reproj_img_index.tab', ]), ])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pdsfile/pds4file/rules/cassini_iss_fring_mosaics_rsfrench2025.py` around lines 90 - 96, The current associations_to_documents TranslatorByRegex maps bundle ids to top-level "documents/..." paths, but this module stores documents inside the bundle under "bundles/.../document/..."; update the regex targets in associations_to_documents (the TranslatorByRegex call) so the right-hand patterns point to the in-bundle document locations (i.e., use patterns that insert "bundles/cassini_iss_fring_mosaics_rsfrench2025.../document..." instead of "documents/...") so associated_abspaths('documents') resolves to the bundle-local document paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pdsfile/pds4file/rules/cassini_iss_fring_mosaics_rsfrench2025.py`:
- Around line 240-247: The archive rule's input regex used in archive_paths with
TranslatorByRegex currently allows 'bundles|metadata|previews|diagrams' but
emits bundle-content directories (data_*, browse_*, document/*), which can
produce archive specs pointing outside actual bundle roots; update the regex in
the TranslatorByRegex entries (the tuple that builds archive_paths) to only
match the 'bundles' root (remove 'metadata|previews|diagrams') for the patterns
that emit bundle-content tarballs, and apply the same restriction to the other
TranslatorByRegex entries referenced in the same block (the later entries
covering lines ~252-309) so all rules that produce data_browse_*,
data_browse_mosaic*, document/* archives are constrained to bundle roots.
---
Duplicate comments:
In `@pdsfile/pds4file/rules/cassini_iss_fring_mosaics_rsfrench2025.py`:
- Around line 90-96: The current associations_to_documents TranslatorByRegex
maps bundle ids to top-level "documents/..." paths, but this module stores
documents inside the bundle under "bundles/.../document/..."; update the regex
targets in associations_to_documents (the TranslatorByRegex call) so the
right-hand patterns point to the in-bundle document locations (i.e., use
patterns that insert
"bundles/cassini_iss_fring_mosaics_rsfrench2025.../document..." instead of
"documents/...") so associated_abspaths('documents') resolves to the
bundle-local document paths.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f15bb46a-a50c-4107-800e-4ab575caaa6a
📒 Files selected for processing (3)
pdsfile/pds4file/rules/cassini_iss_fring_mosaics_rsfrench2025.pypdsfile/pds4file/rules/cassini_uvis_solarocc_beckerjarmak2023.pypdsfile/pds4file/rules/uranus_occs_earthbased.py
…l browse products under browse category and all index files under metadata category
…COISS_xxxx.py to move index files from the supplemental to the miscellaneous directory. This change improves organization and consistency in file categorization for Cassini ISS F Ring mosaic data.
…eflect the relocation of index files from the supplemental to the miscellaneous directory, ensuring improved organization and consistency.
… reflect consistent naming for reprojected images, enhancing clarity in file categorization.
…ages by adding logic for handling '_suppl' suffix. Update test results to reflect consistent naming for reprojected images, ensuring accurate categorization and retrieval.
…eprojected images, ensuring accurate root name extraction for label files.
rfrenchseti
requested changes
Apr 13, 2026
| (r'bundles/cassini_iss_fring_mosaics_rsfrench2025/cassini_iss_fring_mosaics_rsfrench2025[^/]*/data_mosaic_bkg_sub/(iss|iosic)_.*/(iss|iosic)_.*metadata.*\.tab', 0, ('Cassini ISS F Ring Mosaics', 40, 'coiss_f_ring_mosaic_bkg_sub_metadata', 'Background-Subtracted Mosaic Metadata', True)), | ||
|
|
||
| # browse_mosaic | ||
| # put all browse files under browse category |
Collaborator
There was a problem hiding this comment.
For the normal Cassini ISS images, we only download the "full" version by default. We should probably do something similar here. I suggest downloading the "medium" version because it's the most useful.
| # document | ||
| (r'bundles/cassini_iss_fring_mosaics_rsfrench2025/cassini_iss_fring_mosaics_rsfrench2025[^/]*/(readme.txt|document/user_guide/.*mosaic.*\.(lblx|pdf))', 0, ('Cassini ISS F Ring Mosaics', 130, 'coiss_f_ring_documentation', 'Documentation', False)), | ||
|
|
||
| # index under document/ |
Collaborator
There was a problem hiding this comment.
Update comment for miscellaneous
| (r'bundles/cassini_iss_fring_mosaics_rsfrench2025/cassini_iss_fring_mosaics_rsfrench2025[^/]*/data_reproj_img/(iss|iosic)_.*/.*_reproj_suppl\.txt', 0, ('Cassini ISS F Ring Reprojected Images', 180, 'coiss_f_ring_reproj_img_spice_pointing', 'Reprojected Image SPICE Pointing', False)), | ||
| (r'bundles/cassini_iss_fring_mosaics_rsfrench2025/cassini_iss_fring_mosaics_rsfrench2025[^/]*/data_reproj_img/(iss|iosic)_.*/.*_reproj_img_metadata_params\.tab', 0, ('Cassini ISS F Ring Reprojected Images', 190, 'coiss_f_ring_reproj_img_metadata', 'Reprojected Image Metadata', False)), | ||
|
|
||
| # browse_reproj_img cassini_iss_fring_mosaics_rsfrench2025 |
Collaborator
There was a problem hiding this comment.
It's kind of confusing that we are mixing mosaics (which have their own opus ids) and reproj images (which are part of the downloads for normal cassini image) in the same opus_types. At the lease you should add a more detailed comment explaining how this works.
| opus_products = translator.TranslatorByRegex([ | ||
| (r'bundles/(cassini_iss_fring_mosaics_rsfrench2025/cassini_iss_fring_mosaics_rsfrench2025[^/]*)/(data|browse)_mosaic.*/((iss|iosic)_[a-zA-Z0-9_]*)/(iss|iosic)_[a-zA-Z0-9_]*_mosaic.*\.[a-z]{3,4}', 0, | ||
| [ | ||
| # bundles data_mosaic/ |
Collaborator
There was a problem hiding this comment.
And same here about an explanatory comment
| r'\1/\2/\2/data_reproj_img', | ||
| r'\1/\2/\2/document/collection_document.csv', | ||
| r'\1/\2/\2/document/collection_document.lblx', | ||
| r'\1/\2/\2/miscellaneous/global_reproj_img_index.lblx', |
Collaborator
There was a problem hiding this comment.
Move below "document/user_guide". Also there is a collection_miscellaneous.
| # For viewables with label (like f ring browse mosaic), we need to get rid of | ||
| # _thumb, _full, _med, and _small from the data file names to obtain the correct | ||
| # root name for the label files. | ||
| if ('_thumb' in self.basename or '_full' in self.basename or |
Collaborator
There was a problem hiding this comment.
This is not a good place for this. Anything specific to particular datasets should be under rules. Don't hardcode special cases in the generic code.
…mproved label file mapping Introduce PRODUCT_LBL_BASENAME_WO_EXT to facilitate the extraction of label file basenames without extensions. Update logic in PdsFile to utilize this new attribute for determining root names. Adjust regex patterns in cassini_iss_fring_mosaics_rsfrench2025.py and test results to reflect changes in file categorization, ensuring consistent handling of browse and background-subtracted images.
Collaborator
Author
|
All comments addressed. |
Introduce a new subclass for the Cassini ISS Spokes dataset, enhancing the Pds4File class with specific regex patterns for product categorization, view options, and archive handling. This addition includes the implementation of various translators for metadata and file organization, ensuring accurate mapping and retrieval of reprojected images and associated documentation.
Add detailed comments to clarify the structure and purpose of archive files and directories for the Cassini ISS Spokes dataset. This includes specifying the inclusion of the entire bundle, data_derived, and browse_derived directories, along with associated files for improved organization and clarity in file categorization.
Updated regex patterns to include new mappings for B Ring and F Ring reprojected images in the COISS_xxxx.py file. Added test results for new image files, ensuring accurate categorization and retrieval of associated metadata. This enhancement improves the organization and clarity of the Cassini ISS dataset.
Incorporated additional test cases in test_pds4file_blackbox.py to support the cassini_iss_spokes_hedman-hamilton-2024 dataset. This includes mappings for documentation and various derived image files, enhancing the coverage and accuracy of the test suite for the Cassini ISS dataset.
…on_2024.py to streamline the code and improve clarity. This cleanup enhances maintainability by eliminating redundant code segments.
…milton_2024.py to streamline the code and enhance maintainability by eliminating redundancy.
…ni_iss_spokes_hedman_hamilton_2024.py to streamline code and improve maintainability.
Collaborator
Author
|
juzen2003
changed the title
…d cassini_iss_spokes_hedman_hamilton_2024.py to include specific descriptors for F Ring and B Ring reprojected images. This enhances clarity in product categorization and improves maintainability of the code.
…o improve product categorization for B Ring reprojected images. Adjusted sort keys for clarity and maintainability.
…to streamline test cases for the cassini_iss_spokes_hedman-hamilton-2024 dataset, enhancing clarity and maintainability of the test suite.
…g reprojected image mappings. Adjusted sort keys for clarity and maintainability, ensuring accurate categorization of associated metadata in the dataset.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixed Add PdsFile support for the F Ring bundle when it becomes available #73
Changes:
cassini_iss_fring_mosaics_rsfrench2025:pds4file/rules/cassini_iss_fring_mosaics_rsfrench2025.pycassini_iss_fring_mosaics_rsfrench2025 bundlecassini_iss_fring_mosaics_rsfrench2025 bundlecassini_iss_fring_mosaics_rsfrench2025 bundleassociations_to_bundlesrules for mosaic files incassini_iss_fring_mosaics_rsfrench2025 bundleassociations_to_previewsrules for mosaic files incassini_iss_fring_mosaics_rsfrench2025 bundledefault_viewablesrules for mosaic files incassini_iss_fring_mosaics_rsfrench2025 bundleopus_productsandassociated_abspathsandopus_id_to_primary_logical_pathopus_idandopus_typeCOISS_xxxx.py:cassini_iss_fring_mosaics_rsfrench2025test_opus_productsthat includes reproj files.opus_productsto handle the existence of cross pds3 and pds4 productsHistory of changes:
cassini_iss_fring_mosaics_rsfrench2025.opus_typeto put all browse products underbrowsecategory and all index files undermetadatacategorycoiss_f_ring_mosaic_metadatacoiss_f_ring_mosaic_bkg_sub_metadatacoiss_f_ring_reproj_img_spice_pointingcoiss_f_ring_reproj_img_metadatacassini_iss_spokes_hedman_hamilton_2024.pyand add rules forcassini_iss_spokes_hedman_hamilton_2024pdsdata/holdingsto pass the tests without shelves for the new test cases:Questions:
previews/directory? Or do we create new opus type for them? (the current output is using the new opus type) Also I assume we want to include files from bundles/browse_mosaic as well, but with different preview opus types. Please review the opus types for these.Summary by CodeRabbit
New Features
Improvements
Tests