Preserve dictionary order in the anonymizer.

Review Request #1781 — Created Feb. 16, 2015 and submitted

benjyw
pants
bd868f3...
pants-reviews
patricklaw
This matters because when splitting zinc analysis files
we must choose a representative class from each source file
(see that code for why), and in some cases we end up choosing
the first class in dictionary order. If the anonymized version
doesn't pick the equivalent representative, tests will fail.

Used this to generate the new test data for https://rbcommons.com/s/twitter/r/1779/. It was a failing test there that alerted me to this complication.

  • 0
  • 0
  • 3
  • 0
  • 3
Description From Last Updated
ST
  1. 
      
  2. src/python/pants/backend/jvm/tasks/jvm_compile/anonymizer.py (Diff revision 1)
     
     
     
     
     
     
     
     

    This may be a convenient violation of the liskov substitution principal, but renaming Anonymizer to Filter would probably improve clarity.

    1. Fair point, I'll go one further and refactor this properly.

  3. 
      
BE
PA
  1. Descending below 10k now, will follow up with more!

  2. An or rather than an (ugly) python ternary would work just as well here.

    1. Not so - an empty wordmap passed into the ctor should not cause us to fall through to the default one.

    2. Ah you're right, whoops.

  3. This might be my favorite use of python's filter I've ever seen.

  4. 
      
ST
  1. 
      
  2. change method name to indicate that it's just reading the file?

    1. Good idea. Done.

  3. 
      
PA
  1. Ship It!
  2. 
      
BE
BE
Review request changed

Status: Closed (submitted)

Change Summary:

Thanks all. Submitted as e44aacaf80c34445c555bce9bb0f3e2e441cf3c7.

Loading...