Refactor/pyamapping

[dreinsch]

We created a new package to share all mapping / conversion related functions in the interactive-sonification packages.
See https://github.com/interactive-sonification/pyamapping
This PR adapts the current code to work with pyamapping

[wiccy46]

e6357ea Doesn't look like it should be part of the PR. Can you double check and rebase your project.

[wiccy46]

We should pin pyamapping rather than just acquiring whatever the latest, this can be dangerous down the road with breaking change. Also I check your pyamapping dependency requirements which has the same concern, I think you should also pin your numpy version.

[wiccy46]

And my last comment is:
What is the main purpose of pyamapping? Is this purely about providing mapping functions. If so for example clip or clipping is not so much of a clipping but already a dsp. If we want to provide a collection easy to recall audio related processing not only limited to unit transformation, then we can consider a bigger scope and maybe a different name.

[dreinsch]

e6357ea Doesn't look like it should be part of the PR. Can you double check and rebase your project.

I rebased the PR and force pushed

[thomas-hermann]

mapping is more general than just scaling, a mapping does not need to be invertible, in this sense a clipping is a mapping, as well as certain nonlinear distortions that are not monotonous. pyamapping should in future contain polynomial functions as well as mappings tailored to a given data distribution, e.g. using the cdf as mapping function.

[thomas-hermann]

note that dbamp, midicps, etc remain possible abbreviations for the (now recommended and more pythonic named functions) db_to_amp and midi_to_cps(). In this sense some changes in pya are not strictly necessary.

[dreinsch]

Thank you for your feedback.

What is the main purpose of pyamapping? Is this purely about providing mapping functions. If so for example clip or clipping is not so much of a clipping but already a dsp. If we want to provide a collection easy to recall audio related processing not only limited to unit transformation, then we can consider a bigger scope and maybe a different name.

The main purpose of pyamapping is to provide a home for all (audio related) functions that are shared between the interactive-sonification packages (pya, sc3nb, mesonic, sonecules). This is done to reduce code duplication and insure that the users always
always use the same interfaces across our packages.

[dreinsch]

We should pin pyamapping rather than just acquiring whatever the latest, this can be dangerous down the road with breaking change.

It does not look like the versions are pinned (==) in a way that would prevent the installation of a newer i.e. scipy.
To my understanding you leave the version as unrestricted as possible as this eases the dependency solving process.
At least this should be the case if you are developing a library and not an end user application for a specific production environment.

Also, for a more detailed reasoning, see this stackoverflow question:
https://stackoverflow.com/questions/28509481/should-i-pin-my-python-dependencies-versions

[wiccy46]

We don't and probably shouldn't pin it with ==, but we should provide at least a lower limit. And probably the next major release as the higher limit. Or else this will be dangerous.

[wiccy46]

This is the same issue (a bigger one) in pyamapping as numpy is not pinned. We had to bump it in the past because there is already a breaking change in np.linspace if i remember correctly. So we should move away from some of the older numpy releases.

[thomas-hermann]

method name change (adding to) not mandatory but useful!

[wiccy46]

I understand this argument, but I would say in this case the naming of pyamapping is limited. I would recommend rethinking this and have a package name that is can encompass a larger scope of functionalities. If no better options show up then we stick to it.

[wiccy46]

I approved the PR, But I would still recommend pinning numpy with a lowest version.

[dreinsch]

Will do. I noticed that the pya dependency on numpy was removed by 47d0758 and currently it is only indirectly dependend on numpy because of scipy. Previously it was numpy>=1.22.0, this is also close to what is currently set in the main branch of scipy numpy>=1.22.4. However in the maintenance/1.7.x branch they specify numpy>=1.16.5,<1.24.0 and I noticed that they have a whole package to deal with this problem: https://github.com/scipy/oldest-supported-numpy. However I can imagine that their code is way more coupled with the specific numpy versions.
So I would suggest using numpy>=1.22.4 for now. Or what would you suggest?

I also think it will be quite likely that in the future (perhaps 0.2.0 already) pyamapping will depend on scipy as well to more easily provide more functionalities as e.g. a data driven mapping. Additionally in the envisioned use cases of pyamapping it will be installed mostly together with pya and thus scipy anyway. So we could wait with this pull request until then.

Regarding your previous comment on the name of pyamapping:

I understand this argument, but I would say in this case the naming of pyamapping is limited. I would recommend rethinking this and have a package name that is can encompass a larger scope of functionalities. If no better options show up then we stick to it.

What would be your suggestions for a better name?
The current name is based on the idea that it is obviously related to pya and that it offers mapping functions.
Additionally using import pyamapping as pam results in a mirrored map
Would a name like pyafun or pyafunctions be better suited in your opinion?

Perhaps a better route alltogether would be to

• keep all these functions in pya
• and move the pyaudio dependency of pya into an extra.

pyaudio is to my knowledge only necessary for the PyAudioBackend for the Aserver and in our use cases we often use Asig instances for signal conditioning and plotting and do not need the Aserver/playback/record functionality. This would allow users interested in these use cases only to use pya without installing portaudio or am I mistaken?

I added @aleneum as reviewer to see what his perspective on this is. But perhaps we should take this discussion elsewhere.

[wiccy46]

I have no strong opinion on which minimum version of numpy to use. So I think your suggested version should be fine. I don't think we need to go way back. Let's stay modern to some extend.

Secondly, yes, making pyaudio an optional dependency, similar to what pydub do is something I always want to see. In many applications, we just want to use pya as a number crunching and visualization tool rather than interfacing with hardware. Besides, the trouble of portaudio has never really enable a simple pip install to work out of the box. But I never got around to do that, if you are willing to take initiative that would be much appreciated. .

[aleneum]

Hello everyone,

I am not really sure what I should review so I will just add my 50 cents after a brief scan of the conversation.

Should functionality be ported into another package?

If the new package will exclusively used by pya for the foreseeable future I'd leave it as it is right now.
If the outsourced functionality will be used by other packages and thus a) redundancy will be reduced, b) the other packages cannot import pya for some reason and c) the increased maintenance effort/version and release juggling will not nullify all saved effort, why not? But maybe chose a name not related to pya then? pya, however, should provide a transparent interface for the outsourced capabilities in case users are meant to interact with them. As a library user I don't have a detailed mental model of the internal structure of pya and the related packages and it's quite annoying when I have to remember where stuff I need right now comes from. Even worse if I used to import something from pya and have to import it from pya-mapping after the next release without a warning/deprecation phase. beautifulsoup comes into my mind where I always have to check which (optional and not yet installed) parser I need for what task. Luckily, 're-exports' are rather easy with Python. Imports from the 'wrong' module can also be an anti pattern though (e.g. import PyAudio from pya.backend.pyaudio 😱).

Make pyaudio optional

Since @dreinsch and @wiccy46 presented use cases where pyaudio is not needed I'd make it optional since it's rather tedious to deal with pyaudio, especially if you don't actually need it.

Pinning versions

There is a guy running around on Github named PyDeps who is (semi-)manually checking project dependencies for too loose or too strict dependencies and he has some valid points. I did a little research and I'd say that as mentioned by @wiccy46 there should be some reasonable version pinning established. We should not forget that a release cannot be undone! == is far too strict and will prevent pya to function for many use cases where Numpy has been pinned to certain version ranges. But no version pinning might make older versions of pya unusable really fast when dependencies introduce breaking changes. Again: There is no going back to "fix" releases. A minimal version is a no brainer and should be chosen depending on the required features and when they had been introduced. A maximum version is a bit more tricky, especially for numpy/scipy. A pragmatic choice would be to pin it to the latest published minor release or -- if you know that the dependency developers will definitely not break anything -- major releases since even though SemVer suggests to patch broken releases, some projects tend to release new (fixed) minor versions instead. The requirement update process could be automated with GHA if necessary. This might require more patch releases but imho that is not a bad thing.

Cheers!