Skip to content

Fix incorrect trajectory_grid vs instrumentation warning in Phrase class #49

@jon-myers

Description

@jon-myers

Summary

The Phrase class validation incorrectly warns when trajectory_grid dimensions don't match instrumentation length. This validation is wrong - instrumentation should be related to phrase_grid dimensions (at the Piece level), not trajectory_grid dimensions.

Current Incorrect Behavior

Location: idtap/classes/phrase.py:287-293

# Validate grid structure consistency
if 'trajectory_grid' in opts and 'instrumentation' in opts:
    trajectory_grid = opts['trajectory_grid']
    instrumentation = opts['instrumentation']
    if len(trajectory_grid) != len(instrumentation):
        import warnings
        warnings.warn(f"trajectory_grid has {len(trajectory_grid)} tracks but instrumentation has {len(instrumentation)} instruments. "
                     "These should typically match.", UserWarning)

Error Example

/Users/jon/.local/share/virtualenvs/DN-Book-Figures-AgsBk-yQ/lib/python3.13/site-packages/idtap/classes/phrase.py:301: UserWarning: trajectory_grid has 2 tracks but instrumentation has 1 instruments. These should typically match.
  warnings.warn(f"trajectory_grid has {len(trajectory_grid)} tracks but instrumentation has {len(instrumentation)} instruments. "

Why This is Wrong

  1. trajectory_grid vs phrase_grid: instrumentation is related to phrase_grid dimensions at the Piece level, not trajectory_grid at the Phrase level
  2. New trajectory structure: As of recent updates, trajectory_grid will have 2 dimensions for both Sitar and Sarangi instruments
  3. Architectural mismatch: The Phrase class doesn't have the same instrumentation concept as the Piece class - phrases can have multi-dimensional trajectory grids independent of piece-level instrumentation

Expected Behavior

This warning should be removed from the Phrase class. The validation doesn't make architectural sense at the phrase level.

If instrumentation validation is needed, it should be at the Piece level where phrase_grid dimensions should match instrumentation length (which already exists in piece.py:418).

Fix

Remove lines 287-293 from idtap/classes/phrase.py:

# DELETE THIS BLOCK - incorrect validation
if 'trajectory_grid' in opts and 'instrumentation' in opts:
    trajectory_grid = opts['trajectory_grid']
    instrumentation = opts['instrumentation']
    if len(trajectory_grid) != len(instrumentation):
        import warnings
        warnings.warn(f"trajectory_grid has {len(trajectory_grid)} tracks but instrumentation has {len(instrumentation)} instruments. "
                     "These should typically match.", UserWarning)

Related Code

Similar (but correct) warnings exist in the same file for chikari_grid and groups_grid (lines 295-310). These should be reviewed to determine if they have the same architectural issue.

The correct instrumentation validation exists in piece.py:418:

if len(phrase_grid) != len(instrumentation):
    warnings.warn(f"phraseGrid has {len(phrase_grid)} tracks but instrumentation has {len(instrumentation)} instruments. "
                 "These should typically match.", UserWarning)

Priority

Medium - This produces incorrect warnings that confuse users, but doesn't break functionality.

Impact

  • Users see spurious warnings during normal usage
  • Warnings are technically incorrect based on the architecture
  • May cause users to think their data is malformed when it's actually correct

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions