On Mon, Nov 30, 2020 at 11:33 PM David Gow <davidgow@xxxxxxxxxx> wrote: > > On Tue, Dec 1, 2020 at 7:33 AM Daniel Latypov <dlatypov@xxxxxxxxxx> wrote: > > > > The use of manual open() and .close() calls seems to be an attempt to > > keep the contents in scope. > > But Python doesn't restrict variables like that, so we can introduce new > > variables inside of a `with` and use them outside. > > > > Do so to make the code more Pythonic. > > > > Signed-off-by: Daniel Latypov <dlatypov@xxxxxxxxxx> > > --- > I'm fine with this, and it clearly works fine for me. Out of > curiosity, though, is there any difference here other than it being > more usual Python style? Files aren't leaked if we error out due to some exception before we call close(). And self.assertEqual() and friends raise exceptions on failure, so we're toeing that line here. But other than being more robust, it should be equivalent. > > We've struggled a bit in the past toeing a line between trying to > follow "normal" Python style versus adapting it a bit to be more > "kernel-y". Experience thus far has actually been that going out on > our own has caused more problems than it solves, so I'm all for this > change, but I do admit that my brain does understand the older code a > touch more easily. Ack. Python's lack of lexical scopes is a bit disconcerting. But from a readability standpoint, it's a bit more self-evident than having to write it would be, imo. So I think following more standard Python style here outweighs the cost. > > In any case, > Reviewed-by: David Gow <davidgow@xxxxxxxxxx> > > > -- David
