feat(timedelta): add scientific notation #3346

JeanArhancet · 2021-10-22T14:22:20Z

Change Summary

There is an issue with scientific notations and timedelta

Related issue number

fix #3315

Checklist

Unit tests for the changes exist
Tests pass on CI and coverage remains at 100%
Documentation reflects the changes where applicable
changes/<pull request or issue id>-<github username>.md file added describing change
(see changes/README.md for details)
My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

JeanArhancet · 2021-10-24T12:47:22Z

please review

pydantic/datetime_parse.py

ehiggs

lgtm!

samuelcolvin

good catch, but I think we can implement in a cleaner way.

samuelcolvin · 2021-12-09T00:02:42Z

pydantic/datetime_parse.py

@@ -226,7 +226,14 @@ def parse_duration(value: StrBytesIntFloat) -> timedelta:
        value = str(value)
    elif isinstance(value, bytes):
        value = value.decode()
-
+
+    if isinstance(value, (str)):


Instead of adding this into the logic for all cases, I think you can match all the cases described in your tests with another regex, which can be appended to the current regexes checked and therefore have no effect on performance if it's not called.

Thanks for you feedback.

The regex in the commit 88ffae7 is good for you?

please review

changes/3315-JeanArhancet.md

samuelcolvin · 2021-12-09T00:05:12Z

please update

Co-authored-by: Samuel Colvin <samcolvin@gmail.com>

JeanArhancet · 2021-12-23T17:11:34Z

please review

samuelcolvin

otherwlise I think looking good.

samuelcolvin · 2021-12-24T13:33:53Z

pydantic/datetime_parse.py

@@ -54,6 +54,9 @@
    r'$'
 )

+# Support scientific notation
+scientific_notation = re.compile(r'^(?P<sign>[-+]?)' r'(?P<scientific_notation>\d+(.\d+)?[eE][+\-]?\d+)?' r'$')


Suggested change

scientific_notation = re.compile(r'^(?P<sign>[-+]?)' r'(?P<scientific_notation>\d+(.\d+)?[eE][+\-]?\d+)?' r'$')

scientific_notation = re.compile(r'^(?P<sign>[-+]?)(?P<scientific_notation>\d+(.\d+)?[eE][+\-]?\d+)?$')

samuelcolvin · 2021-12-24T13:35:57Z

pydantic/datetime_parse.py

@@ -243,6 +248,9 @@ def parse_duration(value: StrBytesIntFloat) -> timedelta:
    if kw.get('seconds') and kw.get('microseconds') and kw['seconds'].startswith('-'):
        kw['microseconds'] = '-' + kw['microseconds']

+    if kw.get('scientific_notation'):
+        kw['seconds'] = ("%.17s" % kw.pop('scientific_notation')).rstrip('0').rstrip('.')


Suggested change

kw['seconds'] = ("%.17s" % kw.pop('scientific_notation')).rstrip('0').rstrip('.')

kw['seconds'] = ("%.17s" % kw.pop('scientific_notation')).rstrip('0').rstrip('.')

this needs a comment to explain what's going on, also no need to use percent subsitution, you can use an f-string or format.

Also, I'm not quite clear what you're trying to do, but I would guess you can do it with standard string formatting, no need for rstrip.

I've changed, What do you think ?

samuelcolvin · 2021-12-24T13:36:25Z

please update.

JeanArhancet · 2021-12-25T17:23:09Z

please review

JeanArhancet · 2022-05-30T13:21:34Z

@samuelcolvin this pull request still valid?

samuelcolvin · 2022-05-30T15:44:41Z

Yes, hopefully this can be included in v1.10.

No idea what's going to happen to this on V2, but I guess we cross that bridge when we get there.

samuelcolvin · 2022-08-04T16:15:55Z

I'm really not sure about this, we don't support this in V2 and adding it to speedate wouldn't be trivial.

We can fix the original issue in #3315 by serialising timedeltas better.

samuelcolvin · 2022-08-05T11:24:01Z

replaced by #4329 which i think is simpler and also avoids adding now features which would not be compatible with V2.

Thanks for the contribution, it was useful in working towards a fix.

feat(timedelta): add scientific notation

34ff7d5

JeanArhancet force-pushed the feat/scientific_notation branch from d889680 to 34ff7d5 Compare October 22, 2021 14:24

github-actions bot added the ready for review label Oct 24, 2021

github-actions bot assigned PrettyWood and samuelcolvin Oct 24, 2021

ehiggs reviewed Nov 7, 2021

View reviewed changes

pydantic/datetime_parse.py Outdated Show resolved Hide resolved

ehiggs mentioned this pull request Nov 9, 2021

Fix timedelta validation for small floats #3250

Closed

5 tasks

refactor(scientific): remove regex and add try/catch

88ffae7

ehiggs approved these changes Nov 12, 2021

View reviewed changes

samuelcolvin reviewed Dec 9, 2021

View reviewed changes

github-actions bot added awaiting author revision and removed ready for review labels Dec 9, 2021

github-actions bot assigned JeanArhancet and unassigned samuelcolvin and PrettyWood Dec 9, 2021

Update changes/3315-JeanArhancet.md

dda3a11

Co-authored-by: Samuel Colvin <samcolvin@gmail.com>

github-actions bot added ready for review and removed awaiting author revision labels Dec 23, 2021

github-actions bot assigned PrettyWood and samuelcolvin and unassigned JeanArhancet Dec 23, 2021

refactor(regex): use regex

c49de2e

JeanArhancet force-pushed the feat/scientific_notation branch from 7e0fa8e to c49de2e Compare December 23, 2021 17:14

samuelcolvin reviewed Dec 24, 2021

View reviewed changes

github-actions bot added awaiting author revision and removed ready for review labels Dec 24, 2021

github-actions bot assigned JeanArhancet and unassigned samuelcolvin and PrettyWood Dec 24, 2021

fix(convert): change convert to scientific

ce48134

JeanArhancet force-pushed the feat/scientific_notation branch from 4b47805 to ce48134 Compare December 25, 2021 17:21

github-actions bot added ready for review and removed awaiting author revision labels Dec 25, 2021

github-actions bot assigned PrettyWood and samuelcolvin and unassigned JeanArhancet Dec 25, 2021

samuelcolvin mentioned this pull request Aug 4, 2022

V1.10 release #4324

Closed

14 tasks

samuelcolvin closed this Aug 5, 2022

JeanArhancet deleted the feat/scientific_notation branch August 5, 2022 11:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(timedelta): add scientific notation #3346

feat(timedelta): add scientific notation #3346

JeanArhancet commented Oct 22, 2021 •

edited by samuelcolvin

JeanArhancet commented Oct 24, 2021

ehiggs left a comment

samuelcolvin left a comment

samuelcolvin Dec 9, 2021

JeanArhancet Dec 9, 2021

ehiggs Dec 22, 2021

samuelcolvin commented Dec 9, 2021

JeanArhancet commented Dec 23, 2021

samuelcolvin left a comment

samuelcolvin Dec 24, 2021

samuelcolvin Dec 24, 2021

JeanArhancet Dec 25, 2021 •

edited

samuelcolvin commented Dec 24, 2021

JeanArhancet commented Dec 25, 2021

JeanArhancet commented May 30, 2022

samuelcolvin commented May 30, 2022

samuelcolvin commented Aug 4, 2022

samuelcolvin commented Aug 5, 2022 •

edited

	scientific_notation = re.compile(r'^(?P<sign>[-+]?)' r'(?P<scientific_notation>\d+(.\d+)?[eE][+\-]?\d+)?' r'$')
	scientific_notation = re.compile(r'^(?P<sign>[-+]?)(?P<scientific_notation>\d+(.\d+)?[eE][+\-]?\d+)?$')

	kw['seconds'] = ("%.17s" % kw.pop('scientific_notation')).rstrip('0').rstrip('.')
	kw['seconds'] = ("%.17s" % kw.pop('scientific_notation')).rstrip('0').rstrip('.')

feat(timedelta): add scientific notation #3346

feat(timedelta): add scientific notation #3346

Conversation

JeanArhancet commented Oct 22, 2021 • edited by samuelcolvin

Change Summary

Related issue number

Checklist

JeanArhancet commented Oct 24, 2021

ehiggs left a comment

Choose a reason for hiding this comment

samuelcolvin left a comment

Choose a reason for hiding this comment

samuelcolvin Dec 9, 2021

Choose a reason for hiding this comment

JeanArhancet Dec 9, 2021

Choose a reason for hiding this comment

ehiggs Dec 22, 2021

Choose a reason for hiding this comment

samuelcolvin commented Dec 9, 2021

JeanArhancet commented Dec 23, 2021

samuelcolvin left a comment

Choose a reason for hiding this comment

samuelcolvin Dec 24, 2021

Choose a reason for hiding this comment

samuelcolvin Dec 24, 2021

Choose a reason for hiding this comment

JeanArhancet Dec 25, 2021 • edited

Choose a reason for hiding this comment

samuelcolvin commented Dec 24, 2021

JeanArhancet commented Dec 25, 2021

JeanArhancet commented May 30, 2022

samuelcolvin commented May 30, 2022

samuelcolvin commented Aug 4, 2022

samuelcolvin commented Aug 5, 2022 • edited

JeanArhancet commented Oct 22, 2021 •

edited by samuelcolvin

JeanArhancet Dec 25, 2021 •

edited

samuelcolvin commented Aug 5, 2022 •

edited