Use set/dict literal syntax #169
Conversation
Every supported version can use this syntax, and it's both faster and clearer.
Codecov Report
@@ Coverage Diff @@
## master #169 +/- ##
=======================================
Coverage 83.85% 83.85%
=======================================
Files 1 1
Lines 539 539
Branches 95 95
=======================================
Hits 452 452
Misses 63 63
Partials 24 24
Continue to review full report at Codecov.
|
cloudpickle/cloudpickle.py
Outdated
| out_names = set(names[oparg] | ||
| for op, oparg in _walk_global_ops(co)) | ||
| out_names = {names[oparg] | ||
| for op, oparg in _walk_global_ops(co)} |
There was a problem hiding this comment.
indentation a bit weird here tho.
The `op` arg isn't used for creating `out_names`, so we elide it.
|
Fixed lint errors. |
HyukjinKwon
left a comment
HyukjinKwon
left a comment
There was a problem hiding this comment.
@alok BTW, mind checking if cloudpickle roughly worked on Python 2.6 too? I know it's something we don't officially support; however, if that worked before and this change breaks it, I doubt if we should fix these only for style reason.
|
set literals were added in 2.7, so this will not work with 2.6. Though, if it is not supported, it is not supported, right? |
|
Correct but I was thinking I wouldn't bother to fix styles if that breaks a potential practical use case. There's no gain here rather than styles but we might lose a practical use case. |
|
dictionary and set comprehensions are good but If there's no gain better than styles, I would like to know if this was already broken with Python 2.6. It wouldn't be too challenging. |
|
@HyukjinKwon This syntax won't work in 2.6.
I think it's worth ignoring 2.6 anyway. Python 2 end-of-life is in 1 year, 7 months, 30 days, 2 hours, and 25 minutes as of this moment. I've yet to see someone use 2.6 these days. Any version of python 2 below 2.7 is not supported by any major library, and the entire scientific python stack is pretty much on 3+ now. Using this as a rough metric, about 0.2% of pypi downloads are for 2.6. That's not a large enough userbase for me to care about a version of python released a decade ago. |
|
Yup, I am aware of the history for Python 2.x and that syntax is specific to 2.7. Note that old OS already has 2.6 built-in Python and probably 2.6 wouldn't be newly installed on purpose much. In my previous company, there were thousands of customers relying on it. I personally think It needs another decade to ignore and push away Python 2.6. I would be fine if there's better cases that could be fixed by something Python 2.7 has specifically but was less sure of these since there's a (unofficial) cost and risk we should take here. For clarification, I am okay if anyone feels fine with it. I don't get in the way. It's -0 from me (Apache way). |
|
I have just merged. Thanks @alok and @llllllllll. |
4 participants
Every supported version can use this syntax, and it's both faster and clearer.