npower-python3-gpo-fix review

Douglas Bagnall douglas.bagnall at
Sat Oct 27 21:16:23 UTC 2018

On 27/10/18 10:07 PM, Noel Power wrote:
> Actually 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.


> 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 '' 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.


More information about the samba-technical mailing list