Skip to content

Improve metadata handling in spectrogram sources and add safety tests#178

Open
tejasnarwade wants to merge 5 commits intosunpy:mainfrom
tejasnarwade:fix-metadata-handling
Open

Improve metadata handling in spectrogram sources and add safety tests#178
tejasnarwade wants to merge 5 commits intosunpy:mainfrom
tejasnarwade:fix-metadata-handling

Conversation

@tejasnarwade
Copy link
Copy Markdown

@tejasnarwade tejasnarwade commented Mar 17, 2026

Summary

This PR improves the robustness of spectrogram source detection by handling cases where metadata keys may be missing.

What was the issue?

Some is_datasource_for implementations accessed metadata using direct dictionary indexing (e.g., meta["instrument"]).
This caused a KeyError when the metadata dictionary did not contain those keys.

What changes were made?

  • Replaced direct dictionary access with safer .get() usage in:
    • EOVSASpectrogram
    • RSTNSpectrogram
  • Added a parametrized test (test_metadata_safety.py) to ensure that all relevant spectrogram sources:
    • Handle missing metadata gracefully
    • Do not raise exceptions

Why this matters

This makes the code more robust when working with incomplete or unexpected metadata, which can occur in real-world data scenarios.

Additional Notes

While adding the test, it revealed that some sources were not handling missing metadata correctly.
This PR fixes those inconsistencies and ensures uniform behavior across sources.

Testing

  • All existing tests pass
  • New tests added for missing metadata edge cases

PR Description
AI tools were used for:

  • Code generation (e.g., when writing an implementation or fixing a bug)
  • Test/benchmark generation
  • Documentation (including examples)
  • Research and understanding
  • No AI tools were used

@samaloney
Copy link
Copy Markdown
Member

Thanks for the PR this seems related to #175 and #177 I think this could all be handled in one PR and see my comment in #175

I see the potential problem you are trying to address, have you actually found a case of it happening with real data?

@tejasnarwade
Copy link
Copy Markdown
Author

when tried with incomplete data it caused keyerror hence i thought to fix it, as many time incomplete data is being entered

Thanks for the PR this seems related to #175 and #177 I think this could all be handled in one PR and see my comment in #175

I see the potential problem you are trying to address, have you actually found a case of it happening with real data?

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.

2 participants