Re: [osinfo-db PATCH] tests, test_dates: Keep just one implementation of _parse_iso_date()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wednesday, 22 May 2019 15:16:27 CEST Fabiano Fidêncio wrote:
> As suggested by Cole, let's keep just one implementation of
> _parse_iso_date() (the most compatible one) and avoid diverging over
> time.
> 
> Signed-off-by: Fabiano Fidêncio <fidencio@xxxxxxxxxx>
> ---
>  tests/test_dates.py | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/tests/test_dates.py b/tests/test_dates.py
> index 49df613..aca7472 100644
> --- a/tests/test_dates.py
> +++ b/tests/test_dates.py
> @@ -8,14 +8,10 @@ import sys
>  from . import util
>  
>  
> -if sys.version_info >= (3, 7):
> -    def _parse_iso_date(date_string):
> -        return datetime.date.fromisoformat(date_string)
> -else:
> -    def _parse_iso_date(date_string):
> -        m = re.match("([0-9]{4})-([0-9]{2})-([0-9]{2})", date_string)
> -        assert m
> -        return datetime.date(int(m.group(1)), int(m.group(2)), int(m.group(3)))
> +def _parse_iso_date(date_string):
> +    m = re.match("([0-9]{4})-([0-9]{2})-([0-9]{2})", date_string)
> +    assert m
> +    return datetime.date(int(m.group(1)), int(m.group(2)), int(m.group(3)))

The purposes of having the two implementation are:
- datetime.date.fromisoformat() is available in newer versions, so it
  makes sense to leverage it
- my implementation is not optimized as datetime.date.fromisoformat();
  considering this is only a test, it is "good enough"
- having the version check means it is easier to drop the old
  implementation when raising the minimum Python version required

So I'm on -1 on this.

-- 
Pino Toscano

Attachment: signature.asc
Description: This is a digitally signed message part.

_______________________________________________
Libosinfo mailing list
Libosinfo@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libosinfo

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Fedora Users]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux