Skip to content

Update pyfar introduction notebook - #173

Merged
mberz merged 2 commits into
developfrom
update/pyfar_introduction
Sep 2, 2026
Merged

Update pyfar introduction notebook#173
mberz merged 2 commits into
developfrom
update/pyfar_introduction

Conversation

@f-brinkmann

@f-brinkmann f-brinkmann commented Sep 1, 2026

Copy link
Copy Markdown
Member

Update to avoid a pyfar depreaction warning and minor improvements.

"The respective links are located at the top of each example notebook. \n",
"\n",
"To follow along with the workshop document, either navigate to [pyfar.org](https://pyfar.org) and scroll down to the **Workshops** section and select the **pyfar introduction**, or scan the following QR code:\n",
"For an interactive version of this notebook visit the [pyfar's example gallery](https://pyfar-gallery.readthedocs.io/en/latest/) and select the **pyfar introduction**, or scan the following QR code:\n",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a workshop anymore, and we renamed the Wokshop section to 'Getting started'. Thought its savest to simply link to the gallery.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"metadata": {},
"outputs": [],
"source": [
"rir_inv = pf.dsp.normalize(rir_inv)\n",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The normalization was a little arbitrary, especially if being done before converting to minimum phase. Not doing this, also shows the effect of the filter more clearly in the plot generated below.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not arbitrary. Without the normalization, the resulting filter attenuates the overall signal, i.e. it reduces the loudness of a signal passed to the filter. This usually is not wanted for equalizers, except if they also calibrate absolute level corresponding to a physical quantity.
I'd suggest to restore the normalization and explanation

Comment on lines +300 to +301
"inverter = pf.dsp.RegularizedSpectrumInversion.from_frequency_range(\n",
" rir_smoothed, frequency_range=frequency_range)\n",

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to new class to avoid a deprecation warning from pyfar

@f-brinkmann f-brinkmann self-assigned this Sep 1, 2026
@f-brinkmann f-brinkmann moved this from Backlog to Require review in Weekly Planning Sep 1, 2026
@f-brinkmann
f-brinkmann requested review from a team, ahms5, hoyer-a, mberz and sikersten September 1, 2026 12:12

@hoyer-a hoyer-a left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one suggestion

"The respective links are located at the top of each example notebook. \n",
"\n",
"To follow along with the workshop document, either navigate to [pyfar.org](https://pyfar.org) and scroll down to the **Workshops** section and select the **pyfar introduction**, or scan the following QR code:\n",
"For an interactive version of this notebook visit the [pyfar's example gallery](https://pyfar-gallery.readthedocs.io/en/latest/) and select the **pyfar introduction**, or scan the following QR code:\n",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@f-brinkmann
f-brinkmann requested a review from hoyer-a September 1, 2026 14:10
@f-brinkmann
f-brinkmann force-pushed the update/pyfar_introduction branch from 335ee8f to 84bce4e Compare September 1, 2026 14:10

@mberz mberz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only have a conceptual comment

"metadata": {},
"outputs": [],
"source": [
"rir_inv = pf.dsp.normalize(rir_inv)\n",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was not arbitrary. Without the normalization, the resulting filter attenuates the overall signal, i.e. it reduces the loudness of a signal passed to the filter. This usually is not wanted for equalizers, except if they also calibrate absolute level corresponding to a physical quantity.
I'd suggest to restore the normalization and explanation

@f-brinkmann

Copy link
Copy Markdown
Member Author

@mberz I reintroduced the normalization in a way that I think generalizes better to other types of filters.

@f-brinkmann
f-brinkmann requested a review from mberz September 2, 2026 08:36
@mberz
mberz merged commit 96da291 into develop Sep 2, 2026
7 checks passed
@github-project-automation github-project-automation Bot moved this from Require review to Done in Weekly Planning Sep 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants