Skip to content

Conversation

@pieleric
Copy link
Member

@pieleric pieleric commented Jan 9, 2026

HDF5 is not commonly used on systems with light, so we never supported
this metadata... but now some SPARCs have input light (for PL), so let's
add support for this metadata too.

HDF5 is not commonly used on systems with light, so we never supported
this metadata... but now some SPARCs have input light (for PL), so let's
add support for this metadata too.
Copilot AI review requested due to automatic review settings January 9, 2026 16:54
@coderabbitai
Copy link

coderabbitai bot commented Jan 9, 2026

📝 Walkthrough

Walkthrough

This pull request adds support for a new metadata field LightPower to the HDF5 data I/O module. The reading functionality in _parse_physical_data extracts the LightPower value from the PhysicalData group and stores it as MD_LIGHT_POWER metadata. The writing functionality in _add_image_metadata serializes the LightPower metadata back to the HDF5 file with appropriate state tracking (ST_REPORTED when present, ST_INVALID when absent). The corresponding test suite was updated to verify correct propagation of this metadata field across multiple test scenarios.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding support for storing MD_LIGHT_POWER metadata in HDF5 files.
Description check ✅ Passed The description provides relevant context explaining why MD_LIGHT_POWER support is being added to HDF5, directly relating to the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 09c5db0 and 151c7f6.

📒 Files selected for processing (2)
  • src/odemis/dataio/hdf5.py
  • src/odemis/dataio/test/hdf5_test.py
🧰 Additional context used
🧬 Code graph analysis (1)
src/odemis/dataio/hdf5.py (2)
src/odemis/model/_components.py (1)
  • model (570-571)
src/odemis/gui/conf/file.py (1)
  • get (125-140)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: CodeQL analysis (python)
  • GitHub Check: build (ubuntu-24.04)
  • GitHub Check: build (ubuntu-22.04)
🔇 Additional comments (5)
src/odemis/dataio/hdf5.py (2)

729-729: LGTM! LightPower reading follows established patterns.

The reading of LightPower metadata is correctly implemented following the same pattern as other similar fields (Magnification, Baseline). Using the float converter and default bad_states is appropriate for optional power measurements.


960-964: LGTM! LightPower writing handles state correctly.

The implementation properly handles the LightPower metadata with appropriate state tracking:

  • Defaults to 0.0 when missing (consistent with Baseline)
  • Sets ST_INVALID state for missing values (filtered on read)
  • Sets ST_REPORTED state for present values
  • Maintains symmetry with the reading code at line 729
src/odemis/dataio/test/hdf5_test.py (3)

517-517: LGTM! Test data includes MD_LIGHT_POWER appropriately.

Adding MD_LIGHT_POWER to the test metadata with a reasonable value (0.5 W). The selective addition (only to the first metadata dict) properly tests both presence and absence scenarios.


565-566: LGTM! Assertion correctly verifies MD_LIGHT_POWER round-trip.

The assertion follows the established pattern for optional metadata (same as MD_LENS_MAG check above). Properly verifies that MD_LIGHT_POWER is preserved through the write/read cycle.


662-663: LGTM! Comprehensive test coverage for MD_LIGHT_POWER.

The assertions are consistently added across multiple test scenarios (temporal spectrum, angular spectrum, AR, time correlation). These tests verify that MD_LIGHT_POWER is correctly handled when present and properly omitted when absent, ensuring robust coverage.

Also applies to: 775-776, 906-907, 1427-1428


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds support for storing and reading MD_LIGHT_POWER metadata in the HDF5 file format. This metadata represents the light power used during acquisition, which is now needed for SPARCs with input light for photoluminescence (PL) measurements.

  • Added MD_LIGHT_POWER metadata reading and writing in the HDF5 format handler
  • Updated test cases to validate the new metadata is correctly preserved during save/load cycles

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/odemis/dataio/hdf5.py Added reading of "LightPower" metadata (line 729) and writing logic for MD_LIGHT_POWER (lines 960-963)
src/odemis/dataio/test/hdf5_test.py Added MD_LIGHT_POWER test data and assertions across multiple test functions to verify proper handling

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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