MAINT: Wave cleanup #5996
MAINT: Wave cleanup #5996
Conversation
combine unnecessary if/else
variable for clarity
@@ master #5996 diff @@
======================================
Files 238 238
Stmts 43768 43803 +35
Branches 8221 8211 -10
Methods 0 0
======================================
+ Hit 34199 34230 +31
- Partial 2601 2603 +2
- Missed 6968 6970 +2
|
|
LGTM. Ready for merge from your end? |
|
yes, if you think they're all good changes. |
|
Yeah, it makes the code look cleaner and read a bit better. I'll merge in a couple of days if nobody else has comments. |
| size = struct.unpack(fmt, fid.read(4))[0] | ||
|
|
||
| # Number of bytes per sample | ||
| bytes = bits//8 |
There was a problem hiding this comment.
It might be worth renaming this variable to something like bytes_per_sample, while we're doing maintenance, to avoid shadowing the builtin name.
|
I made one comment inline, but I think some of the places where comments were added might be better served by just giving the variable(s) a better name. That said, this is a good change regardless, so I'm +1 with or without better variable names. |
That's what I was going to do, but then thought maybe it was important to keep the original names, since they're also argument names. I'll change them if you think it's a good idea. |
|
Making the names more readable makes sense to me |
move comments about meanings into docstring
|
ok I did that |
| @@ -170,7 +195,7 @@ def read(filename, mmap=False): | |||
|
|
|||
| Returns | |||
| ------- | |||
| rate : int | |||
| fs : int | |||
| Sample rate of wav file as a Python integer. | |||
There was a problem hiding this comment.
don't need as a Python integer
There was a problem hiding this comment.
Ok, just moved it from the notes section
|
Ok changed |
|
LGTM. @perimosocordiae you happy now? |
|
LGTM, merged. Thanks, @endolith! |
I started to look into supporting 24-bit WAV files, but decided I don't care enough now that I know about PySoundFile. :)
Here are some changes I made in the process of reading through the code. Just changed an error message, added comments, renamed a variable, and got rid of an unnecessary if/else.