npower-python3-gpo-fix review

[Douglas Bagnall]

On 27/10/18 10:07 PM, Noel Power wrote:
> Actually io.open seems the correct choice, I somehow got confused, it really was too early in the morning :-)

Right. So "from io import open" is like a "from __future__" line for Py3
open behaviour, and we should really be doing it everywhere.

>      def write_binary(self, filename):
> -        if PY3:
> -            kwargs = {'encoding': self.encoding}
> -        else:
> -            kwargs={}
> -        with open(filename, 'w', **kwargs) as f:
> +       from io import open
> +        with open(filename, 'w', self.encoding) as f:

Yes, this is good. RB+ with this.

We need to look at moving the import to the top, making it file-wide,
but I'm happy for that to be a later patch.

> So this looks good (if we are happy with the other changes). I'm out monday so if you are happy with the above I'll rewrite my commits with this change and the other minor
> tweaks from review when I return.

Excellent.

> Also I'd bet we are are using 'w' or 'r' with other 'open' calls all over the code base that could do with some 'io.open' love
> 

Yes. I count 212 open()s in 81 python files, two with existing "from io import open".
I'm hoping we don't need to think about each one individually.

Thank you.

Douglas