Skip to content

feat: Add trackTitles support to Piece class (Issue #44)#45

Merged
jon-myers merged 1 commit intomainfrom
feature/issue-44-track-titles
Nov 6, 2025
Merged

feat: Add trackTitles support to Piece class (Issue #44)#45
jon-myers merged 1 commit intomainfrom
feature/issue-44-track-titles

Conversation

@jon-myers
Copy link
Contributor

Summary

Implements support for the trackTitles attribute in the Piece class, enabling custom labeling of instrument tracks for better identification in multi-track recordings.

Resolves #44

Background

The main IDTAP project added trackTitles in commit 770c1ef4 (Sept 2024), but the Python API was not updated. This caused ValueError when loading transcriptions containing the trackTitles attribute.

Use Case

Particularly useful for recordings with multiple instances of the same instrument:

  • Sarangi trios: "Lead Melody", "Harmony", "Drone"
  • Multiple vocalists: "Soloist", "Chorus"
  • Multi-track recordings with distinct roles

Implementation Details

1. Parameter Validation (piece.py:217-224)

  • Added 'trackTitles' to allowed constructor parameters

2. Type Validation (piece.py:348-353)

  • Validates trackTitles is a list
  • Ensures all items are strings

3. Initialization (piece.py:105-117)

  • Defaults to empty strings array matching instrumentation length
  • Accepts explicit trackTitles from constructor
  • Automatic synchronization:
    • Pads with empty strings if shorter than instrumentation
    • Truncates if longer than instrumentation

4. Serialization (piece.py:1336)

  • Includes trackTitles in to_json() output

Testing

New Test Coverage (9 tests)

✅ Default initialization with empty strings
✅ Explicit values provided
✅ Length synchronization (shorter array → padded)
✅ Length synchronization (longer array → truncated)
✅ Type validation (non-list raises TypeError)
✅ Type validation (non-string items raise TypeError)
✅ Serialization round-trip preservation
✅ Empty string values handling
✅ Real-world sarangi trio use case

Test Results

  • All 356 tests pass (including 9 new trackTitles tests)
  • Full backward compatibility maintained
  • No breaking changes to existing functionality

Example Usage

from idtap import Piece
from idtap.enums import Instrument

# Sarangi trio with custom track labels
piece = Piece({
    'instrumentation': [Instrument.Sarangi, Instrument.Sarangi, Instrument.Sarangi],
    'trackTitles': ['Lead Melody', 'Harmony', 'Drone'],
    'phrases': [phrase1, phrase2, phrase3],
    'raga': raga
})

# Access track titles
print(piece.track_titles)  # ['Lead Melody', 'Harmony', 'Drone']

# Serialization preserves titles
json_data = piece.to_json()
# json_data['trackTitles'] == ['Lead Melody', 'Harmony', 'Drone']

Backward Compatibility

Fully backward compatible

  • Existing code without trackTitles continues to work
  • Defaults to empty strings for each instrument track
  • No changes required to existing client code

Checklist

  • Implementation matches TypeScript version behavior
  • All parameter validation in place
  • Comprehensive test coverage (9 new tests)
  • All existing tests pass (356 total)
  • Serialization/deserialization working
  • Documentation in test docstrings
  • Backward compatibility maintained

🤖 Generated with Claude Code

Resolves Issue #44

Add support for the trackTitles attribute to enable custom labeling of
instrument tracks in multi-track recordings. This feature is particularly
useful for recordings with multiple instances of the same instrument
(e.g., sarangi trios with "Lead Melody", "Harmony", and "Drone" labels).

## Changes:
- Add 'trackTitles' to allowed parameters in Piece constructor
- Initialize track_titles with automatic length synchronization
- Add type validation (must be list of strings)
- Include trackTitles in serialization (to_json)
- Maintain backward compatibility (defaults to empty strings)

## Testing:
- 9 comprehensive test cases covering:
  - Default initialization
  - Explicit values
  - Length synchronization (padding/truncation)
  - Type validation
  - Serialization round-trip
  - Real-world use cases (sarangi trio)
- All 356 existing tests continue to pass

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2025

📦 Test Package Built Successfully!

This PR has been automatically built and uploaded to TestPyPI for testing.

🔗 TestPyPI Link: https://test.pypi.org/project/idtap/

To test this version:

pip install --index-url https://test.pypi.org/simple/ idtap

✅ All tests passed and package builds successfully.

@jon-myers
Copy link
Contributor Author

@claude review

@claude
Copy link
Contributor

claude bot commented Nov 6, 2025

Claude finished @jon-myers's task —— View job


Code Review Complete ✅

Overview

This is a well-implemented feature that adds trackTitles support to the Piece class. The implementation is thorough, well-tested, and maintains backward compatibility.

Strengths

Code Quality & Best Practices

  • Clean implementation: Code follows existing patterns and conventions in the codebase
  • Proper parameter validation: Added to _validate_parameters() method with clear error messages (piece.py:362-367)
  • Consistent naming: Uses track_titles (snake_case) internally, trackTitles (camelCase) for serialization
  • Good documentation: Clear docstrings in test functions explaining behavior

Robust Implementation

  • Smart initialization logic: Automatically synchronizes array length with instrumentation (piece.py:113-117)
  • Defensive copying: Creates copy of input list to prevent external mutations (piece.py:108)
  • Proper type validation: Ensures list type and all string elements (piece.py:364-367)
  • Serialization support: Correctly included in to_json() method (piece.py:1336)

Excellent Test Coverage

  • Comprehensive test suite: 9 new tests covering all edge cases
  • Real-world scenarios: Includes sarangi trio use case from the issue
  • Validation testing: Tests both positive and negative cases
  • Round-trip testing: Ensures serialization/deserialization works correctly

🔍 Minor Observations

Code Quality

  1. Redundant validation (piece.py:364-367): The type validation is duplicated - first checks if it's a list, then checks again in the string validation. Could be streamlined but doesn't affect functionality.

  2. Missing edge case handling: What happens if instrumentation is empty? The current logic would create an empty track_titles array, which is probably correct but worth considering.

Performance Considerations

  • Minimal impact: The synchronization loops (piece.py:114-117) are O(n) where n is typically small (number of instrument tracks)
  • Memory efficient: Creates defensive copy only when needed

Security

  • No security concerns: Simple string array handling with proper type validation
  • Input sanitization: Validates all inputs before processing

🎯 Specific Code Review

Initialization Logic (piece.py:105-117)

# Initialize trackTitles (after instrumentation is set)
track_titles = opts.get('trackTitles')
if track_titles is not None:
    self.track_titles = list(track_titles)  # Create a copy
else:
    # Create empty strings for each instrument track
    self.track_titles = [''] * len(self.instrumentation)

# Ensure trackTitles array matches instrumentation length
while len(self.track_titles) < len(self.instrumentation):
    self.track_titles.append('')
while len(self.track_titles) > len(self.instrumentation):
    self.track_titles.pop()

✅ Excellent: Handles all cases (None, shorter, longer, exact match) with clear, readable logic.

Parameter Validation (piece.py:362-367)

# Validate trackTitles
if 'trackTitles' in opts and opts['trackTitles'] is not None:
    if not isinstance(opts['trackTitles'], list):
        raise TypeError(f"Parameter 'trackTitles' must be a list, got {type(opts['trackTitles']).__name__}")
    if not all(isinstance(title, str) for title in opts['trackTitles']):
        raise TypeError("All items in 'trackTitles' must be strings")

✅ Good: Clear error messages and proper type checking.

Test Quality

The test suite is exemplary:

  • Tests default behavior, explicit values, length synchronization
  • Validates error conditions with proper exception matching
  • Includes real-world use case (sarangi trio)
  • Tests serialization round-trip

📋 Final Assessment

Category Rating Notes
Code Quality ⭐⭐⭐⭐⭐ Clean, readable, follows conventions
Test Coverage ⭐⭐⭐⭐⭐ Comprehensive, covers all cases
Performance ⭐⭐⭐⭐⭐ Minimal overhead, efficient
Security ⭐⭐⭐⭐⭐ No concerns, proper validation
Backward Compatibility ⭐⭐⭐⭐⭐ Fully maintained

🚀 Recommendation

APPROVE - This is production-ready code that solves the stated problem effectively. The implementation is robust, well-tested, and maintains full backward compatibility. The feature will resolve the compatibility issue with the main IDTAP project while providing valuable functionality for multi-track recordings.

The code demonstrates excellent software engineering practices and would be a valuable addition to the codebase.


@jon-myers jon-myers merged commit e5630ff into main Nov 6, 2025
1 check passed
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.

Add support for trackTitles attribute in Piece class

1 participant