Skip to content
This repository has been archived by the owner on Jan 31, 2022. It is now read-only.

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

Closed
1 of 3 tasks
bdorney opened this issue Jun 27, 2019 · 4 comments
Closed
1 of 3 tasks

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

bdorney opened this issue Jun 27, 2019 · 4 comments

Comments

@bdorney
Copy link
Contributor

bdorney commented Jun 27, 2019

Brief summary of issue

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

Types of issue

  • Bug report (report an issue with the code)
  • Feature request (request for change which adds functionality)
  • Maintanence

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
Copy link
Contributor

jsturdy commented Jun 27, 2019

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
Copy link
Contributor Author

bdorney commented Jun 27, 2019

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:

if ohKey not in calInfo.keys():
calAdcCalFile = "{0}/{1}/calFile_{2}_{1}.txt".format(dataPath,chamber_config[ohKey],adcName)
calAdcCalFileExists = os.path.isfile(calAdcCalFile)

(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
Copy link
Contributor

jsturdy commented Jun 27, 2019

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
Copy link
Contributor

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.