convert all % formatted strings under src/ to str.format format

Review Request #2042 — Created April 7, 2015 and submitted

nhoward_tw
pants
1378
a7ead6d...
pants-reviews
areitz, ity, jsirois, zundel

For the most part this is a straight forward conversion. The only tricky part is where %d was used on arguments that may be floats. Currently, python's float doesn't support the d format, so I had to change those locations so that the argument was coerced to an int before rendering.

Used the intellij python conversion hint + regex search replace to remove extra number / type info. After that, checked each conversion by hand.

ran ci.sh locally and CI passed: https://travis-ci.org/pantsbuild/pants/builds/57533316

  • 0
  • 0
  • 6
  • 0
  • 6
Description From Last Updated
NH
ST
  1. Would be good to include some information about how you did this (presumably not manually?)

  2. 
      
DT
  1. 
      
  2. src/python/pants/base/validation.py (Diff revision 1)
     
     

    does this need to use !s for formatting the exception?

  3. 
      
DT
  1. for pages 5 and 6

  2. src/python/pants/goal/products.py (Diff revision 1)
     
     

    use !s instead of str()?

    1. Hmm. I'm leaning towards doing the opposite. !s is the default and %s has the same behavior as {} for exceptions. So, I don't need to explicitly say {!s} anywhere.

  3. src/python/pants/goal/run_tracker.py (Diff revision 1)
     
     

    ditto !s?

  4. 
      
NH
ZU
  1. I didn't find any problems. Did you make these changes by hand or use a tool?

  2. Its ok, but I did a double take on the old version: too many %%%

  3. funny how we use !r in some places and repr() on the argument in others.

    1. yeah, I think that's funny too. But for this, I'm trying to keep things pretty mechanical.

  4. 
      
JS
  1. Thanks for doing this!
    
    The only issues I found were bad conversions of float formatting - all noted.  I'm sure I missed some >100 but flagged several.
    It looks like you got the tricky bits like { escaping, % un-escaping and tuple/kwargs splatting.
  2. outdent 3
  3. Nice!  Even the commented out code will work ({{) when/if uncommented!
  4. And unescapes!
  5. src/python/pants/base/validation.py (Diff revision 1)
     
     
    >100
  6. src/python/pants/base/validation.py (Diff revision 1)
     
     
    >100
  7. src/python/pants/base/validation.py (Diff revision 1)
     
     
    >100
  8. src/python/pants/cache/cache_setup.py (Diff revision 1)
     
     

    This is a change in output, though I think its probably fine:

    >>> '%3f' % .4
    '0.400000'
    >>> '{:3}'.format(.4)
    '0.4'
    

    If you want to match the old behavior (which was buggy too, should have been %.3f IIUC):

    >>> '{:.5f}'.format(.4)
    '0.40000'
    
    1. Erm, or actually {:.6f}

    2. I'll use .6f. The format DSL is funkier than I'd supposed, I'd expected that .3 would work by itself, but the type defaults to d and blows up. :/

  9. src/python/pants/net/http/fetcher.py (Diff revision 1)
     
     

    Should be {:.3f}

  10. Should be {:.3f} for this one and the next 2 below.

  11. Should be {timing:.3f}

  12. 
      
IT
  1. Looks good from the first few pages I looked at! as Eric asked, I had the same question -- would be good to share what tool/script you used for this.

    1. I used IntelliJ. I did a project search for %[sd], and ['"] % , then used keyboard shortcuts to run through the conversion intention for each instance. After that, I cleaned up the resulting output with a search replace--the IntelliJ Python plugin's conversion isn't perfect, but it's consistent.

  2. 
      
NH
JS
  1. Ship It!
  2. 
      
NH
Review request changed

Status: Closed (submitted)

Change Summary:

merged at 9514990ef3d6e24aaa747e96ed4069cf4e43824a

Loading...