[MRG] Discard phase before amplitude_to_db in docs + update warning in *_to_db

[lostanlen]

Reference Issue

Fixes #723.
Since PR #671 (which closed issue #662), the example code for specshow uses librosa.amplitude_to_db on the output of stft, which raises a UserWarning in the absence of a magphase transformation.
Searching for _to_db( reveals the same problem in over 20 example codes, in tutorial notebooks, and docstring.

What does this implement/fix? Explain your changes.

As an example, the previous docstring for STFT was:

>>> librosa.display.specshow(librosa.power_to_db(D), y_axis='log', x_axis='time')

This PR replaces the above with the following:

>>> D_dB = librosa.power_to_db(np.abs(D)**2, ref=np.max)
>>> librosa.display.specshow(D_dB, y_axis='log', x_axis='time')

Any other comments?

I had previously submitted PR #742 to fix the same issue, but during the review process @bmcfee and I realized that it would be simpler, and therefore better, to just use np.abs rather than librosa.magphase() when possible.

---

This change is 

[stefan-balke]

---

librosa/core/spectrum.py, line 1041 at r1 (raw file):

    >>> y, sr = librosa.load(librosa.util.example_audio_file())
    >>> CQT = librosa.cqt(y, sr=sr, fmin=librosa.note_to_hz('A1'))

To align this to the other examples, we should use C, rather then CQT, or?

[stefan-balke]

---

librosa/util/utils.py, line 1308 at r1 (raw file):

    >>> y, sr = librosa.load(librosa.util.example_audio_file())
    >>> tempo, beats = librosa.beat.beat_track(y=y, sr=sr, trim=False)
    >>> cqt = np.abs(librosa.cqt(y=y, sr=sr))

Here the same as above

[stefan-balke]

---

librosa/core/spectrum.py, line 105 at r1 (raw file):

    >>> y, sr = librosa.load(librosa.util.example_audio_file())
    >>> D = np.abs(librosa.stft(y))

This looks weird to me, shouldn't we do D = librosa.stft(y) and then in a new line D = np.abs(D) for the sake of clarity? Holds true for all changes and up for discussion.

[stefan-balke]

---

librosa/core/spectrum.py, line 107 at r1 (raw file):

    >>> D = np.abs(librosa.stft(y))
    >>> D
    array([[  2.576e-03 -0.000e+00j,   4.327e-02 -0.000e+00j, ...,

Why is this still a complex dtype?

[stefan-balke]

Reviewed 10 of 10 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @lostanlen)

[stefan-balke]

@lostanlen thanks for this update and in general this is super. Just have a more style-related question in the review. Would be fine with any solution.

[lostanlen]

@stefan-balke thank you for the CR. You are right that there are some naming consistency issues that aren't addressed, plus, what is worse, an array D that should not be complex in the doctring. Will address

[lostanlen]

Reviewable status: 9 of 10 files reviewed, 4 unresolved discussions (waiting on @stefan-balke and @lostanlen)

---

librosa/core/spectrum.py, line 105 at r1 (raw file):

Previously, stefan-balke (Stefan Balke) wrote…

This looks weird to me, shouldn't we do D = librosa.stft(y) and then in a new line D = np.abs(D) for the sake of clarity? Holds true for all changes and up for discussion.

Meh. Let's not fix what isn't broken

---

librosa/core/spectrum.py, line 107 at r1 (raw file):

Previously, stefan-balke (Stefan Balke) wrote…

Why is this still a complex dtype?

fixed in d4c2cf5

---

librosa/core/spectrum.py, line 1041 at r1 (raw file):

Previously, stefan-balke (Stefan Balke) wrote…

To align this to the other examples, we should use C, rather then CQT, or?

fixed in 90e112d

[lostanlen]

Reviewable status: 9 of 10 files reviewed, 4 unresolved discussions (waiting on @stefan-balke and @lostanlen)

---

librosa/util/utils.py, line 1308 at r1 (raw file):

Previously, stefan-balke (Stefan Balke) wrote…

Here the same as above

do you have a good variable name in mind?

[lostanlen]

Reviewable status: 9 of 10 files reviewed, 4 unresolved discussions (waiting on @stefan-balke)

---

librosa/core/spectrum.py, line 107 at r1 (raw file):

Previously, lostanlen (Vincent Lostanlen) wrote…

fixed in d4c2cf5

Done.

---

librosa/core/spectrum.py, line 1041 at r1 (raw file):

Previously, lostanlen (Vincent Lostanlen) wrote…

fixed in 90e112d

Done.

[lostanlen]

@stefan-balke my round of revisions is complete. Thanks again for the CR. I incorporated the changes in naming conventions and updated the numerical output in the stft docstring

That said, I'm not sold on what you argued earlier:

shouldn't we do D = librosa.stft(y) and then in a new line D = np.abs(D) for the sake of clarity?

so I'm leaving this part on hold.

[stefan-balke]

---

librosa/util/utils.py, line 1308 at r1 (raw file):

Previously, lostanlen (Vincent Lostanlen) wrote…

do you have a good variable name in mind?

For consistency reasons I would go with C

[lostanlen]

Reviewable status: 8 of 10 files reviewed, 4 unresolved discussions (waiting on @stefan-balke and @lostanlen)

---

librosa/util/utils.py, line 1308 at r1 (raw file):

Previously, stefan-balke (Stefan Balke) wrote…

For consistency reasons I would go with C

Fixed in 7dc9d64

[lostanlen]

Reviewable status: 8 of 10 files reviewed, 4 unresolved discussions (waiting on @stefan-balke)

---

librosa/util/utils.py, line 1308 at r1 (raw file):

Previously, lostanlen (Vincent Lostanlen) wrote…

Fixed in 7dc9d64

Done.

[stefan-balke]

Reviewed 1 of 1 files at r2.
Reviewable status: 9 of 10 files reviewed, 1 unresolved discussion (waiting on @stefan-balke and @lostanlen)

[stefan-balke]

[stefan-balke]

Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @lostanlen)

[lostanlen]

Thanks again @stefan-balke for the CR. This is ready for merging. On to librosa v0.6.2!

[bmcfee]

Reviewed 7 of 10 files at r1, 1 of 1 files at r2, 1 of 1 files at r3.
Reviewable status: complete! all files reviewed, all discussions resolved

[bmcfee]

Thanks @lostanlen and @stefan-balke !