search

bastibe / PySoundCard

PySoundCard is an audio library based on PortAudio, CFFI and NumPy
BSD 3-Clause "New" or "Revised" License
87 stars 9 forks source link

Miscellaneous Changes #30

Closed mgeier closed 10 years ago

mgeier commented 10 years ago

This pull request collects a few rather minor changes (w.r.t. the amount of code that's changed) and I didn't want to make a PR for each of them.

Some of them are innocuous, others might cause some discussion.

If certain commits turn out to be controversial, I can move them to separate pull requests.

bastibe commented 10 years ago

Most of these are great!

One exception: The sample rate and block size renames are backwards incompatible changes. We better have very good reasons for that. We have several hundred users and no way of notifying them of breaking changes. I think we should back away from those, even though the new names are better.

mgeier commented 10 years ago

I'm well aware that those changes are backwards-incompatible. I think it's worth breaking backwards compatibility between version 0.5 and 0.6. These renames are just the tip of the iceberg, I think there may be several more backwards-incompatible changes coming.

BTW, I did already at least one breaking change in da487ac5ba0587d2905a76c0d64ddee2938cc5b5 (incompatible change to callback signature).

bastibe commented 10 years ago

Let's wait and see that change in context then. Please separate that one change out into a different PR or postpone it to The Big Refactor, so we can get the small things out of the way first.

Regarding the callback signature change: Yes, but that change did make using the callback easier and faster, so it was well-justified. This change right now is just changing some names without much benefit.

mgeier commented 10 years ago

This change right now is just changing some names without much benefit.

The benefit would be that it doesn't annoy the hell out of me each time I'm reading one of those names.

Anyway, I removed the commits and I'll try to endure my suffering for a little time longer.

"Set default blocksize to 0" is also a breaking change, but it was just very wrong and I need it for future changes, so I'll keep that commit.

"Make frames obligatory in read()" is also a breaking change, but I guess nobody used the (nearly equally bad) default value ever anyway, so it shouldn't be a problem.

Any more comments?

If not, feel free to merge.

bastibe commented 10 years ago

Just to be clear here, I totally agree with the rename. It's just that it is not justified on its own. I'm sure we will do a big refactor soon, which will change many things. If we have to break the method signature anyway for other reasons, we can rename these two as well.