More explicit code for segment slicing in dejittering

[cboulay]

I also added a warning when effective srate was more than 10% different from specified srate.

[cbrnr]

I think this looks good. +1 for merge.

Some general comments:

1. I'd prefer if everyone made PRs from a dedicated branch in their fork. This makes it easier for others to pull remote branches. This also means no branches other than master in this repo (upstream).
2. Our code should try to adhere to PEP8. I know especially the line length of 79 characters is sometimes a problem, but in most cases it is possible.
3. At least 2 members of the dev team should have seen a PR. Ideally, if a member makes a PR, another member should perform the merge (except of course for trivial changes like typos and whatnot).

WDYT? We could create a short how to contribute guide with these points. Do you have anything to add?

[cbrnr]

Re this PR: do you want to add an entry to CHANGELOG.md? This refactoring could be in a CHANGED section.

[cboulay]

I'd prefer if everyone made PRs from a dedicated branch in their fork. This makes it easier for others to pull remote branches. This also means no branches other than master in this repo (upstream).

Sorry but I'm a maintainer on many different open source repos and it would be a nightmare to manage if I had personal forks for each of them. This is especially problematic for repos like this that are also submodules because I guarantee at some point I would end up having the parent repo pointing its submodule ref to my personal repo. I'll just delete the branch after merge.

Our code should try to adhere to PEP8. I know especially the line length of 79 characters is sometimes a problem, but in most cases it is possible.

I try to adhere to all aspects of PEP8 except the 79 character length. There's already quite a bit of debate about whether or not that should still be included. I'll try to do it for this project but I'm not going to change my IDE defaults on my 6 different computers, so I'll probably miss it sometimes.

At least 2 members of the dev team should have seen a PR. Ideally, if a member makes a PR, another member should perform the merge (except of course for trivial changes like typos and whatnot).

I agree with the second sentence. For the first sentence, sometimes you're going to be waiting months for me to reply to something fairly trivial and that's probably worse than having to revert something after the fact. If you think I was too hasty in merging a PR then let me know and we can work on adding proper unit tests for whatever I missed.

I won't ever make a tagged release without getting consensus so 99.999% of users (who rely on pypi) won't see any changes until well after we've had time to play with changes. If you want to do a develop/master model then that works too.

It's already hard enough for me to find time to work on these things, please don't add any more barriers. Community is more important than having a pristine repo.

[cbrnr]

Sorry but I'm a maintainer on many different open source repos and it would be a nightmare to manage if I had personal forks for each of them. This is especially problematic for repos like this that are also submodules because I guarantee at some point I would end up having the parent repo pointing its submodule ref to my personal repo. I'll just delete the branch after merge.

I have personal forks for most of the repos I contribute to, but it's OK if you want to work on branches in this repo.

I try to adhere to all aspects of PEP8 except the 79 character length. There's already quite a bit of debate about whether or not that should still be included. I'll try to do it for this project but I'm not going to change my IDE defaults on my 6 different computers, so I'll probably miss it sometimes.

That's OK. If I spot any long lines in any PRs I review I will try to shorten them if it's not too much work.

I agree with the second sentence. For the first sentence, sometimes you're going to be waiting months for me to reply to something fairly trivial and that's probably worse than having to revert something after the fact. If you think I was too hasty in merging a PR then let me know and we can work on adding proper unit tests for whatever I missed.

This comment wasn't intended to criticize something you did, it was just an idea which I thought made sense to implement. Having said that, if you agree that at least two devs should have seen a PR, this means that for the majority of PRs (which are made by a dev) another dev will have to review it.

I won't ever make a tagged release without getting consensus so 99.999% of users (who rely on pypi) won't see any changes until well after we've had time to play with changes. If you want to do a develop/master model then that works too.

No, that's fine, let's continue making releases based on consensus.

It's already hard enough for me to find time to work on these things, please don't add any more barriers. Community is more important than having a pristine repo.

I certainly didn't want to add more barriers. I thought that by providing some guidelines maintaining should be less of a burden, but we don't have to implement all or any of my suggestions.

[cbrnr]

Thanks @cboulay!