cms-gem-daq-project / gem-plotting-tools

Repository for GEM commissioning plotting tools
GNU General Public License v3.0
1 stars 26 forks source link

[Maintenance/Refactoring] Unify String Formatting In All Files #228

Closed bdorney closed 4 years ago

bdorney commented 5 years ago

Brief summary of issue

Spurred by discussion with @jsturdy we should unify string formatting to use str.format rather than % operator.

Types of issue

Expected Behavior

All string formating and file I/O operations with formatted strings should default to using str.format rather than % operator.

Current Behavior

Older code blocks still have statements like:

"Hi I'm %i and %f and %s"%(1, 2.0, "string")

Instead of using str.format, e.g.

"Hi I'm {0} and {1} and {2}".format(1, 2.0, "string"))

Context (for feature requests)

Consistency.

jsturdy commented 5 years ago

Instead of using str.format, e.g.

"Hi I'm {0} and {1} and {2}".format(1, 2.0, "string"))

This is python2.6 compatibility oriented, which I don't feel like making an effort to support. In python2.7 the index is not required

In any event, the person implementing this should definitely read about how to control the formatting.

bdorney commented 5 years ago

Instead of using str.format, e.g.

"Hi I'm {0} and {1} and {2}".format(1, 2.0, "string"))

This is python2.6 compatibility oriented, which I don't feel like making an effort to support. In python2.7 the index is not required

In any event, the person implementing this should definitely read about how to control the formatting.

I'm not so sure we should abandon indexing blindly as a "this is py2.6" reason; there are certain instances in which the indexing is specifically needed to get the string sequence correct:

https://github.com/cms-gem-daq-project/gem-plotting-tools/blob/135f4d8221333a0c886772414df50ceef2511893/utils/anautilities.py#L155-L157

(just the first example I found, here indexing is mandatory).

I would rather say that indexing can be dropped when it is not needed.

jsturdy commented 5 years ago

I would rather say that indexing can be dropped when it is not needed.

indeed, this was the point of my last comment. a simple migration from the "%s"%(blah) to "{}".format(blah) doesn't care about indexing in >python2.7, but more complicated formatting can be done, which was not possible before

lpetre-ulb commented 4 years ago

Some (all?) of this formatting has been done in #252. The remaining statements do not deserve a fix in legacy.