Fix/wfm waveform type detection #114

tekperson · 2025-09-03T14:19:35Z

Proposed Changes

This PR addresses two main areas:

1. WFM Type Detection Fix

Fixed issue where digital and IQ waveform files were incorrectly identified as analog waveforms
Implemented proper type detection logic in WFM format classes:
- WaveformFileWFMDigital: Detects digital waveforms by data_type == 6
- WaveformFileWFMIQ: Detects IQ waveforms by the presence of IQ-specific metadata fields
- WaveformFileWFMAnalog: Acts as a fallback for non-digital, non-IQ waveforms

2. Metadata System Usability Improvements

Enhanced error messages with helpful guidance for custom metadata access
Added set_custom_metadata() convenience method for easier metadata management
Improved remap() method with better warnings and documentation
Added comprehensive docstrings to all metadata classes with practical examples
Enhanced file format compatibility documentation

Testing

Verified backwards compatibility maintained
Basic functionality tests pass
Changes tested locally to ensure proper operation

Checklist

Notes

Maintains 100% backwards compatibility
Addresses usability issues identified
Ready for review and testing

… for Python 3.9+

…ersion, better field separation, and enhanced metadata preservation

…d waveform.py. Handle all fields and ensure clean output.

- Add proper _check_metadata() methods to each WFM format class - Digital waveforms identified by data_type == 6 (DataTypes.DIGITAL) - IQ waveforms identified by presence of IQ-specific metadata fields - Analog waveforms as default case when neither digital nor IQ - Prevents all format classes from returning True for any valid WFM file - Resolves issue where digital and IQ waveforms were incorrectly read as analog

- Enhanced error messages with helpful guidance for custom metadata - Added set_custom_metadata() convenience method for easier metadata management - Improved remap() method with better warnings and documentation - Added comprehensive docstrings to all metadata classes and methods - Enhanced WaveformMetaInfo, AnalogWaveformMetaInfo, DigitalWaveformMetaInfo, and IQWaveformMetaInfo - Added detailed field-level documentation for extended_metadata - Included practical examples and file format compatibility notes - Maintains 100% backwards compatibility Addresses usability issues identified in suggestions.md while keeping the API stable.

- Added entries for WFM type detection fix - Added entries for metadata system usability improvements - Documented new set_custom_metadata() method - Documented enhanced error messages and docstrings

nfelt14

The test marked as xfail needs that marker removed, it should pass now.

CHANGELOG.md

nfelt14 · 2025-09-03T14:53:47Z

src/tm_data_types/datum/waveforms/analog_waveform.py

+    - interpreter_factor: Factor used for data interpretation
+    - real_data_start_index: Index where real data starts
+    
+    Example:


The proper docstring syntax is Examples:

Suggested change

Example:

Examples:

nfelt14 · 2025-09-03T14:53:58Z

src/tm_data_types/datum/waveforms/digital_waveform.py

+    - digital_probe_0_name through digital_probe_7_name: Names of digital probes
+    - digital_probe_0_unit through digital_probe_7_unit: Units for digital probes
+    
+    Example:


Suggested change

Example:

Examples:

nfelt14 · 2025-09-03T14:54:17Z

src/tm_data_types/datum/waveforms/digital_waveform.py

+    for digital probe states and digital signal information.
+    
+    Digital-specific fields:
+    - digital_probe_0_state through digital_probe_7_state: State of digital probes


What about 16-bit digital probes?

nfelt14 · 2025-09-03T14:54:32Z

src/tm_data_types/datum/waveforms/iq_waveform.py

+    - iq_window_type: Type of window function used
+    - iq_sample_rate: Sample rate of the IQ signal
+    
+    Example:


Suggested change

Example:

Examples:

nfelt14 · 2025-09-03T15:01:36Z

src/tm_data_types/datum/waveforms/waveform.py

+        raise an AttributeError with helpful guidance on how to add the field.
+    """
+
+    def __getattr__(self, name: str) -> Any:


Is this for the entire dataclass or just the extended metadata? Won't this break normal getattr calls on the dataclass? What about the attributes that aren't part of the extended metadata?

Thinking specifically about subclasses.

nfelt14 · 2025-09-03T15:02:25Z

src/tm_data_types/datum/waveforms/waveform.py

+            raise AttributeError(
+                f"{type(self).__name__} object has no attribute {name!r}. "
+                f"Available custom metadata keys: {available_keys}. "
+                f"To add this field: meta_info.extended_metadata['{name}'] = your_value"


Suggested change

f"To add this field: meta_info.extended_metadata['{name}'] = your_value"

f"To add this field use: meta_info.extended_metadata['{name}'] = your_value"

nfelt14 · 2025-09-03T15:04:07Z

src/tm_data_types/files_and_formats/wfm/wfm.py

+        remapped = self.META_DATA_TYPE.remap(self._META_DATA_LOOKUP.inverse, formatted_data.meta_data)
+
+        # Convert bytes to strings for string-like metadata
+        def convert_bytes_to_str(value):


This might be useful as a helper function outside this function.

nfelt14 · 2025-09-03T15:07:19Z

src/tm_data_types/files_and_formats/wfm/data_formats/analog.py


+    # Reading
+    def _check_metadata(self, meta_data: Dict[str, Any]) -> bool:
+        """Check if metadata indicates this is an analog waveform.


I would recommend encapsulating this behavior into a helper function that returns an enum. Then that single function can be called in multiple places and the output checked to determine the boolean return value for this function.

Something like this:

return get_wfm_type(meta_data) == WfmType.ANALOG

nfelt14 · 2025-09-03T15:09:01Z

src/tm_data_types/datum/waveforms/waveform.py

+
        return remapped_dict

+    extended_metadata: Optional[Dict[str, Any]] = None


Can you move the dataclass attribute above the methods? That is a more standard organization for dataclasses.

nfelt14 · 2025-09-03T22:00:12Z

This branch needs to be rebased on top of main

tekperson added 7 commits May 29, 2025 13:56

Fix: allow optional/unknown metadata keys and improve type conversion…

097b680

… for Python 3.9+

Fix: allow optional/unknown metadata keys and improve type conversion…

ee6bb97

… for Python 3.9+

Improve metadata handling in WFM file format: Add byte-to-string conv…

dcdd840

…ersion, better field separation, and enhanced metadata preservation

Fix: robust metadata remapping and clean up debug prints in wfm.py an…

003c205

…d waveform.py. Handle all fields and ensure clean output.

Update CHANGELOG.md with Phase 1 improvements

36302df

- Added entries for WFM type detection fix - Added entries for metadata system usability improvements - Documented new set_custom_metadata() method - Documented enhanced error messages and docstrings

tekperson requested a review from a team as a code owner September 3, 2025 14:19

auto-assign bot assigned tekperson Sep 3, 2025

nfelt14 requested changes Sep 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix/wfm waveform type detection #114

Fix/wfm waveform type detection #114

tekperson commented Sep 3, 2025

Uh oh!

nfelt14 left a comment

Uh oh!

Uh oh!

nfelt14 Sep 3, 2025

Uh oh!

nfelt14 Sep 3, 2025

Uh oh!

nfelt14 Sep 3, 2025

Uh oh!

nfelt14 Sep 3, 2025

Uh oh!

nfelt14 Sep 3, 2025

Uh oh!

nfelt14 Sep 3, 2025

Uh oh!

nfelt14 Sep 3, 2025

Uh oh!

nfelt14 Sep 3, 2025

Uh oh!

nfelt14 Sep 3, 2025

Uh oh!

nfelt14 Sep 3, 2025

Uh oh!

nfelt14 commented Sep 3, 2025

Uh oh!

Uh oh!

	f"To add this field: meta_info.extended_metadata['{name}'] = your_value"
	f"To add this field use: meta_info.extended_metadata['{name}'] = your_value"


		return remapped_dict

		extended_metadata: Optional[Dict[str, Any]] = None

Fix/wfm waveform type detection #114

Are you sure you want to change the base?

Fix/wfm waveform type detection #114

Conversation

tekperson commented Sep 3, 2025

Proposed Changes

1. WFM Type Detection Fix

2. Metadata System Usability Improvements

Testing

Checklist

Notes

Uh oh!

nfelt14 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nfelt14 commented Sep 3, 2025

Uh oh!

Uh oh!