Closed srh closed 1 year ago
Code Climate has analyzed commit 405cd5d9 and detected 0 issues on this pull request.
View more on Code Climate.
Important update. Much appreciated you moving this forward.
I don't remeber why I changed collections.abc.Callable in my PR. It clearly wasn't work for my application, I agree with the one step at a time approach.
I don't remeber why I changed collections.abc.Callable in my PR. It clearly wasn't work for my application, I agree with the one step at a time approach.
Looking again, I don't like the approach of assigning over collections.abc.Callable
in some other library either -- what I liked about it was that it handled just the three classes that we used without renaming a module to another common module name. I'd probably do something like this for the three classes that we use:
try:
Callable = collections.abc.Callable
except AttributeError:
Callable = collections.Callable
Actually, this would also make me happy, both importing the module as "colls" -- the problem is the confusion of using an existing module name.
if sys.version_info < (3, 3):
# python < 3.3 uses collections
import collections as colls
else:
# but collections is deprecated from python >= 3.3
import collections.abc as colls
I don't think we should touch it at all though, because (a) there isn't active maintenance that, through higher productivity, justifies minor cleanups like this, and (b) there is another branch that probably would get worse merge conflicts.
I didn't do it. I guess Github does that automatically.
@srh no worries
I don't understand why it's not working... @stephanelsmith can you review it and if ok approve it? Maybe it requires your review..
I've just rebased and force-pushed; I'll override the review and manually merge it.
This cherry-picks a change from #284 (thanks @stephanelsmith) and then removes commented lines.
This PR depends on #296, because it would conflict with its
loop
parameter removal changes (which is also redundant with changes in #284).(Why not just merge #284 instead of these? Well, it is bigger than necessary. It commits ql2_pb2.py, it has some signal.signal changes I haven't looked at and understood yet, and it makes some changes around collections.abc that I think are a good cleanup, but I simply don't want to touch if not necessary.)
Checklist