use str.format notation#318
Conversation
|
Thanks a lot for creating this PR! Can you elaborate why you think this is important? As a general piece of advice, even if the change is minor, I always try to put a few words in the PR description so that at least it leaves some tracks about why some changes were made. About the change you are proposing: I think this is the kind of minor cosmetic change that is not really worth the effort and the risk of breaking something without realising. Full disclosure: I am saying that even if my personal preference is to use I see that you have been involved in quite a few PRs in dask-jobqueue recently. I would be quite curious to hear about how you use dask-jobqueue if you don't mind! |
|
Hi @lesteve, I think it is better to use this notation and (at this point) is more standard than the archaic I don't see how this could break anything. I don't use I am also trying to get inspiration from this package for another package of mine |
I agree with you on this as I said (edit: nitpick I would use "older" rather than "archaic" since the latter is unlikely to make a discussion more productive) but I think this is a very personal opinion and I know some very competent people that use
Hence the "risk of breaking something without realising" in my previous message ;-). A few examples off the top of my head: a Overall IMO the benefit/cost is very rarely good enough for making this kind of cosmetic changes especially if you add the cost of arguing/bike-shedding over it ...
Interesting use case, thanks for the details! |
|
As someone who uses the archaic syntax I have no problem with updates to
use `.format`. People seem to prefer it today. I would caution people
though that there is a rewrite going on now. So this may not be the right
time to make changes like these. They may be lost.
…On Tue, Aug 20, 2019 at 5:19 AM Loïc Estève ***@***.***> wrote:
Hi @lesteve <https://github.com/lesteve>, I think it is better to use
this notation and (at this point) is more standard than the archaic "%s" %
x notation.
I agree with you on this as I said but I think this is a very personal
opinion and I know some very competent people that use it as their default
formatting option. Maybe they got to know Python when % based formatting
was the main formatting option, maybe they learned C before Python I
can't tell for sure.
I don't see how this could break anything.
Hence the "risk of breaking something without realising" in my previous
message ;-). A few examples off the top of my head: a %r that is replaced
by {} or maybe .format is forgotten. These kind of behaviour change tend
to slip through code reviews (this is quite tedious to review many small
changes like this) and tests (at the moment our testing in dask-jobqueue
definitely has some room for improvement).
Overall IMO the benefit/cost is very rarely good enough for making this
kind of cosmetic changes especially if you add the cost of
arguing/bike-shedding over it ...
I am also trying to get inspiration from this package for another package
of mine adaptive-scheduler, with which one can run adaptive (somehwat)
interactive calculations on ~50.000 cores, which right now only works well
with SLURM.
Interesting use case, thanks for the details!
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#318?email_source=notifications&email_token=AACKZTHN6H372JMVQCN26U3QFPOLXA5CNFSM4INBJDVKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4WCT5A#issuecomment-522988020>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACKZTDTGMY4BNB6I4FPPHLQFPOLXANCNFSM4INBJDVA>
.
|
|
I am going to close this one. I think #370 is going to remove a lot of this |
|
With #373 f-strings would become available! 🎉 |
4 participants
No description provided.