Re: [gentoo-portage-dev] [PATCH v2] bin/doins.py: implement install -p option (bug 642632)

2017-12-30 Thread Zac Medico
On 12/30/2017 01:45 PM, Mike Gilbert wrote:
> On Fri, Dec 29, 2017 at 7:19 PM, Zac Medico  wrote:
>> +def _set_timestamps(source_stat, dest):
>> +   """Apply timestamps from source_stat to dest.
>> +
>> +   Args:
>> +   source_stat: stat result for the source file.
>> +   dest: path to the dest file.
>> +   """
>> +   os.utime(dest, (source_stat.st_atime, source_stat.st_mtime))
>> +
>> +
>> +if sys.version_info >= (3, 3):
>> +   def _set_timestamps_ns(source_stat, dest):
>> +   os.utime(dest, ns=(source_stat.st_atime_ns, 
>> source_stat.st_mtime_ns))
>> +
>> +   _set_timestamps_ns.__doc__ = _set_timestamps.__doc__
>> +   _set_timestamps = _set_timestamps_ns
>> +
>> +
> 
> This seems weirdly complex. I guess the goal was to reduce the
> sys.version_info check to once per import?

Yeah, and generally it's cleanest to create compatibility shims in cases
like this.

> The __doc__ trick is nifty, but I'm not sure I would ever want to use it 
> myself.

Yeah, the alternative of writing the docstring twice wasn't very appealing.

> Anyway, just my thoughts as an unpracticed python programmer. It looks
> like this code will get the job done.

-- 
Thanks,
Zac



Re: [gentoo-portage-dev] [PATCH v2] bin/doins.py: implement install -p option (bug 642632)

2017-12-30 Thread Mike Gilbert
On Fri, Dec 29, 2017 at 7:19 PM, Zac Medico  wrote:
> +def _set_timestamps(source_stat, dest):
> +   """Apply timestamps from source_stat to dest.
> +
> +   Args:
> +   source_stat: stat result for the source file.
> +   dest: path to the dest file.
> +   """
> +   os.utime(dest, (source_stat.st_atime, source_stat.st_mtime))
> +
> +
> +if sys.version_info >= (3, 3):
> +   def _set_timestamps_ns(source_stat, dest):
> +   os.utime(dest, ns=(source_stat.st_atime_ns, 
> source_stat.st_mtime_ns))
> +
> +   _set_timestamps_ns.__doc__ = _set_timestamps.__doc__
> +   _set_timestamps = _set_timestamps_ns
> +
> +

This seems weirdly complex. I guess the goal was to reduce the
sys.version_info check to once per import?

The __doc__ trick is nifty, but I'm not sure I would ever want to use it myself.

Anyway, just my thoughts as an unpracticed python programmer. It looks
like this code will get the job done.