Removed unused variables etc. #22160
Conversation
| @@ -181,8 +181,6 @@ def subset_font(font_in, font_out, unicodes, opts): | |||
|
|
|||
|
|
|||
| def getsubset(subset, font_in): | |||
| subsets = subset.split('+') | |||
There was a problem hiding this comment.
Looking at the code below, my guess is that if 'menu' in subset should probably have been if 'menu' in subsets and so on.
There was a problem hiding this comment.
Yes, it probably was, but it works since subset is a string.
Tried to make a minimal change (and tried to search for the original code, but to no avail).
There was a problem hiding this comment.
+1 for changing the code below (or leaving as is for now with a comment if you don't dare to change it).
This code has been unchanged since it's addition in 2015, so there is no history to dig up.
Anyway, it's pretty obvious that this was an oversight and only accidentally worked. Checking subsets below is more clear, should be marginally faster, and is more safe in theory (even though it shouldn't make any practical difference for real use cases.
There was a problem hiding this comment.
OK! I'll change it. Both ways should work and using subsets is indeed probably more clear.
There was a problem hiding this comment.
The if 'lang' in subset would be true if just lang-ext were in subset, but not now that we're checking subsets (which is correctly split). I don't know if this is a thing that might happen or if it's even going to be a problem, but this tool is internal anyway.
There was a problem hiding this comment.
Yes, you are correct. One can probably find such cases.
2115b32 to
1e07fdb
Compare
| @@ -7488,7 +7488,8 @@ def cohere(self, x, y, NFFT=256, Fs=2, Fc=0, detrend=mlab.detrend_none, | |||
| """ | |||
| cxy, freqs = mlab.cohere(x=x, y=y, NFFT=NFFT, Fs=Fs, detrend=detrend, | |||
| window=window, noverlap=noverlap, | |||
| scale_by_freq=scale_by_freq) | |||
| scale_by_freq=scale_by_freq, sides=sides, | |||
There was a problem hiding this comment.
My code analysis tool said that sides and pad_to was unused. I assume that they should be passed on (as otherwise these will not have any effect when calling cohere on an Axes).
There was a problem hiding this comment.
Yes, that should be passed on. Seems it never worked, but nobody ever complained... You can correct this.
PR Summary
Some things pointed out by LGTM. Redundant assignments, unused variables etc.
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).