Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions .agents/skills/warnings-and-environment/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
---
name: warnings-and-environment
description: >-
How music21 tells users about problems: `warnings.warn` with a module Warning
class, `environLocal.warn`, and `environLocal.printDebug`. Read before adding
a warning or debug message, and before writing a test or doctest that
triggers one — tests must never print warnings to the console.
---

# Warnings and the Environment

## Three channels

| Call | Goes to | Can a caller catch or silence it? |
|---|---|---|
| `warnings.warn(MusicXMLWarning('...'), stacklevel=2)` | Python's warnings system | Yes: filters, `catch_warnings`, `assertWarns` |
| `environLocal.warn('...')` | `sys.stderr.write`, every time | No (only by redirecting stderr) |
| `environLocal.printDebug('...')` | stderr, only when debug is on | Toggle with `music21/test/toggleDebug.py` |

`environLocal = environment.Environment('moduleName')` at the top of the module.

## Where things stand

Not unified. In 2026 the code has about 73 `environLocal.warn`, 41
`warnings.warn` (some with a module class, some bare), 800 `printDebug`, and no
`logging`. We are moving slowly toward Python's warnings and logging systems.
It is low priority: convert a call when you are already changing it, never as
a sweep.

For new code, prefer `warnings.warn` for anything a caller might reasonably want to catch,
silence, or test. By default Python shows each distinct message once per call
site, so a message that names the object (`f'{p.nameWithOctave} is ...'`) shows
for each different object. Use `environLocal.warn` only when the user must hear
it every time.

Never `print()`.

## Warning classes

A module with its own warnings defines one class subclassing `UserWarning`
(`MusicXMLWarning` in `musicxml/xmlObjects.py`, `TranslateWarning` in
`midi/translate.py`). Warnings are not exceptions, so they do not subclass
`Music21Exception`.

Set `stacklevel` so the reported line is in the caller's code, not ours: 2 from
the method the user called, more if the warning comes from a helper.

## Tests

In a unittest, catch it and check it:

```python
with self.assertWarnsRegex(MusicXMLWarning, 'C-1'):
mxPitch = MEX.pitchToXml(pitch.Pitch('C', octave=-1))
```

## Doctests

doctest captures only stdout, so any warning goes straight to the console
of whoever runs the tests. Every doctest that triggers a warning must catch it.

When the warning is the point of the example, record it and show it.
`simplefilter('always')` keeps Python from swallowing a repeat of a warning an
earlier test already raised:

```
>>> import warnings
>>> with warnings.catch_warnings(record=True) as w:
... warnings.simplefilter('always') #_DOCS_HIDE
... musicxml.m21ToXml.moveOctaveIntoMusicxmlRange(12, 'C12')
9
>>> w[0].message
MusicXMLWarning('C12 is out of MusicXML range; writing octave 9')
```

When it is incidental, hide the silencing from the docs (see `writing-docs` for
`#_DOCS_HIDE`/`#_DOCS_SHOW`):

```
>>> import warnings #_DOCS_HIDE
>>> with warnings.catch_warnings(): #_DOCS_HIDE
... warnings.simplefilter('ignore') #_DOCS_HIDE
... out = midi.translate.timeSignatureToMidiEvents(ts) #_DOCS_HIDE
>>> #_DOCS_SHOW out = midi.translate.timeSignatureToMidiEvents(ts)
```

`catch_warnings` cannot stop `environLocal.warn`. If a doctest has to trigger
one, wrap it in `contextlib.redirect_stderr(io.StringIO())`.
1 change: 1 addition & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
- No `print()`. Use `environLocal = environment.Environment('moduleName')` and
`environLocal.printDebug(...)`, or `environLocal.warn(...)` when the user should hear
about it every time. `test/toggleDebug.py` switches debug output on and off.
See the `warnings-and-environment` skill.
- New exceptions subclass `exceptions21.Music21Exception`.
- Deprecate a function or method with `@common.deprecated(...)`. Never put it on a
property: IDEs read every property while inspecting an object, so the warning would
Expand Down
39 changes: 35 additions & 4 deletions music21/musicxml/m21ToXml.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
# Christopher Ariza
# Jacob Tyler Walls
#
# Copyright: Copyright © 2010-22 Michael Scott Asato Cuthbert
# Copyright: Copyright © 2010-26 Michael Scott Asato Cuthbert
# License: BSD, see license.txt
# ------------------------------------------------------------------------------
'''
Expand Down Expand Up @@ -241,6 +241,32 @@ def _setTagTextFromAttribute(
return subElement


def moveOctaveIntoMusicxmlRange(octave: int, name: str) -> int:
'''
Move an octave into MusicXML's 0-9 range, warning (with `name`) if it had to move.

>>> musicxml.m21ToXml.moveOctaveIntoMusicxmlRange(4, 'C4')
4
>>> import warnings
>>> with warnings.catch_warnings(record=True) as w:
... warnings.simplefilter('always') #_DOCS_HIDE
... musicxml.m21ToXml.moveOctaveIntoMusicxmlRange(12, 'C12')
9
>>> w[0].message
MusicXMLWarning('C12 is out of MusicXML range; writing octave 9')

* New in v11.
'''
if 0 <= octave <= 9:
return octave
newOctave = min(max(octave, 0), 9)
warnings.warn(
MusicXMLWarning(f'{name} is out of MusicXML range; writing octave {newOctave}'),
stacklevel=3,
)
return newOctave


class GeneralObjectExporter:
'''
Packs any `Music21Object` into a well-formed score and exports
Expand Down Expand Up @@ -4392,7 +4418,8 @@ def restToXml(self, r: note.Rest):
tempPitch = pitch.Pitch()
tempPitch.diatonicNoteNum = restObjectPseudoDNN
mxDisplayStep.text = tempPitch.step
mxDisplayOctave.text = str(tempPitch.octave)
mxDisplayOctave.text = str(
moveOctaveIntoMusicxmlRange(tempPitch.octave, f'{r} display'))

return mxNote

Expand Down Expand Up @@ -4580,7 +4607,9 @@ def pitchToXml(self, p: pitch.Pitch):
if p.accidental is not None:
mxAlter = SubElement(mxPitch, 'alter')
mxAlter.text = str(common.numToIntOrFloat(p.accidental.alter))
_setTagTextFromAttribute(p, mxPitch, 'octave')
_setTagTextFromAttribute(
p, mxPitch, 'octave',
transform=lambda o: moveOctaveIntoMusicxmlRange(o, p.nameWithOctave))
return mxPitch

def unpitchedToXml(self,
Expand Down Expand Up @@ -4637,7 +4666,9 @@ def unpitchedToXml(self,

mxUnpitched = Element('unpitched')
_setTagTextFromAttribute(up, mxUnpitched, 'display-step')
_setTagTextFromAttribute(up, mxUnpitched, 'display-octave')
_setTagTextFromAttribute(
up, mxUnpitched, 'display-octave',
transform=lambda o: moveOctaveIntoMusicxmlRange(o, up.displayName))

# within <note>, only <grace>, <cue>, and <chord> may precede
# <unpitched>, so insert after those rather than trying to list
Expand Down
18 changes: 17 additions & 1 deletion music21/musicxml/test_m21ToXml.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from music21 import layout
from music21 import meter
from music21 import note
from music21 import pitch
from music21 import repeat
from music21 import spanner
from music21 import stream
Expand All @@ -33,7 +34,7 @@
from music21.musicxml import helpers
from music21.musicxml import testPrimitive
from music21.musicxml.m21ToXml import (
GeneralObjectExporter, ScoreExporter,
GeneralObjectExporter, MeasureExporter, ScoreExporter,
MusicXMLWarning, MusicXMLExportException
)
from music21.musicxml.xmlToM21 import MeasureParser
Expand Down Expand Up @@ -229,6 +230,21 @@ def testLowVoiceNumbers(self):
xmlOut = self.getXml(m)
self.assertIn('<voice>hello</voice>', xmlOut)

def testOctaveOutOfRange(self):
MEX = MeasureExporter()
with self.assertWarnsRegex(MusicXMLWarning, 'C-1'):
mxPitch = MEX.pitchToXml(pitch.Pitch('C', octave=-1))
self.assertEqual(mxPitch.findtext('octave'), '0')
with self.assertWarnsRegex(MusicXMLWarning, 'G#12'):
mxPitch = MEX.pitchToXml(pitch.Pitch('G#', octave=12))
self.assertEqual(mxPitch.findtext('octave'), '9')

up = note.Unpitched()
up.displayOctave = -2
with self.assertWarns(MusicXMLWarning):
mxNote = MEX.unpitchedToXml(up)
self.assertEqual(mxNote.findtext('unpitched/display-octave'), '0')

def testUnpitchedGraceInVoice(self):
'''
<unpitched> must precede <voice> even on a grace note, which has
Expand Down
Loading