* time.py, added iso 8601 support #130

velle · 2023-07-09T11:36:22Z

Fixes #129.

time.py, added new function _parseiso(value)* relevant functions now call value = _parseiso(value)
wrote inline documentation, and short subparagraph to README.md
have not tested code in any way except the existing pytests which all succeeded
tox gives mypy errors. I was able to remove some, but not all.
tox gives lint errors. Have not figured out how to see what causes the errors.
tox gives griffe error, ast.BoolOp, but did so even before I edited code
not sure if Im using tox correctly. I simply run 'tox'

* relevant functions now call value = _parseiso(value) * wrote inline documentation, and short subparagraph to README.md * have not tested code in any way except the existing pytests which all succeeded * tox gives mypy errors. I was able to remove some, but not all. * tox gives lint errors. Have not figured out how to see what causes the errors. * tox gives griffe error, ast.BoolOp, but did so even before I edited code * not sure if Im using tox correctly. I simply run 'tox'

codecov · 2023-07-09T13:05:56Z

Codecov Report

Merging #130 (d5eb5ac) into main (a8f8119) will increase coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head d5eb5ac differs from pull request most recent head 32014ab. Consider uploading reports for the commit 32014ab to get more accurate results

@@            Coverage Diff             @@
##             main     #130      +/-   ##
==========================================
+ Coverage   99.18%   99.20%   +0.01%     
==========================================
  Files           9        9              
  Lines         737      754      +17     
==========================================
+ Hits          731      748      +17     
  Misses          6        6

Flag	Coverage Δ
macos-latest	`97.61% <100.00%> (+0.19%)`	⬆️
ubuntu-latest	`97.61% <100.00%> (+0.05%)`	⬆️
windows-latest	`96.15% <100.00%> (+0.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
src/humanize/time.py	`100.00% <100.00%> (ø)`
tests/test_time.py	`100.00% <100.00%> (ø)`

... and 1 file with indirect coverage changes

hugovk

Thank you for the PR!

time.py, added new function _parseiso(value)* relevant functions now call value = _parseiso(value)

👍

wrote inline documentation, and short subparagraph to README.md

👍

have not tested code in any way except the existing pytests which all succeeded

Good the existing tests pass :)

Please can you also add tests? Edit test_time.py and for each function we're adding the extra function call, find the corresponding test_* test function. Then each one has a list of test input data inside @pytest.mark.parametrize(). Let's add some example ISO strings to cover the three different try/except blocks in the new function.

tox gives mypy errors. I was able to remove some, but not all.

tox gives lint errors. Have not figured out how to see what causes the errors.

We're only getting mypy errors from lint. Let's come back to these later.

tox gives griffe error, ast.BoolOp, but did so even before I edited code

Yep, we can ignore these, it's the docs build and we get the same on main and it's not actually failing it:

docs: commands[0]> mkdocs build
INFO     -  mkdocstrings: DEPRECATION: mkdocstrings' watch feature is deprecated in favor of MkDocs' watch feature, see https://www.mkdocs.org/user-guide/configuration/#watch
INFO     -  Cleaning site directory
INFO     -  Building documentation to directory: /home/runner/work/humanize/humanize/site
ERROR    -  griffe: .tox/docs/lib/python3.11/site-packages/humanize/time.py:1: Failed to get annotation expression from Tuple: <class 'ast.BoolOp'>
ERROR    -  griffe: .tox/docs/lib/python3.11/site-packages/humanize/time.py:1: Failed to get annotation expression from Tuple: <class 'ast.BoolOp'>
INFO     -  Documentation built in 1.06 seconds
.pkg: _exit> python /opt/hostedtoolcache/Python/3.11.4/x64/lib/python3.11/site-packages/pyproject_api/_backend.py True hatchling.build
  docs: OK (24.70=setup[[23](https://github.com/python-humanize/humanize/actions/runs/5468468419/jobs/9956152679#step:5:24).20]+cmd[1.50] seconds)
  congratulations :) ([24](https://github.com/python-humanize/humanize/actions/runs/5468468419/jobs/9956152679#step:5:25).79 seconds)

I'll check them some other time.

not sure if Im using tox correctly. I simply run 'tox'

Yep, tox is the usual way to run it.

To run a single thing with tox, you can do tox -e py311 for example (where e = tox environment) to only run tests on Python 3.11, or tox -e lint to run lint, including mypy.

And tox -p auto runs all in parallel.

src/humanize/time.py

hugovk · 2023-07-09T17:16:19Z

README.md

+
+Relies on the native `fromisoformat` function that exists on date, datetime and time objects.
+```


Please can you move the final triple backticks after '10 seconds from now'?

They indicate where the special code formatting ends.

For example, right now it looks like:

https://github.com/velle/humanize/blob/iso8601/README.md#also-accepts-iso8601-text-strings

so I removed the ability for _isoparse() to parse into datetime.time. I edited inline documentation and README.md accordingly.

velle · 2023-07-09T20:23:38Z

Hi Hugo. First of all, thanks for the feedback and for helping me learn this :) I will get back to the other points in your feedback later, but right now I need to figure out how to handle what is described below.

I started writing tests (some hours ago), and was almost done. But then I realized that humanize does not support datetime.time at all. I had written _parse_iso to also parse into datetime.time, when possible.

Now I have removed everything that has to do with datetime.time, and edited the documentation accordingly.

And I have committed and pushed this. This commit only removes the irrelevant time code. It does not include the tests I wrote. Since I imagined that you had to evaluate my commits, before ... merging? ... accepting? them into this iso8601 branch (or into this pull request?). The essence is: I decided to make this commit so it only concerns the removal of datetime.time.

Should I do more about this commit? Will this commit (and future commits) automatically be part of this pull request, until this pull request is closed or merged?

hugovk · 2023-07-09T20:59:17Z

This PR isn't accepted or merged yet (see the green "Open" at the top).

Yes, you can commit more things to your branch, and when you push them, they will automatically be part of this PR.

Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>

for more information, see https://pre-commit.ci

velle · 2023-07-24T07:59:48Z

Hi Hugo. As you noticed, I have been gone, first because of issues with my laptop, and then a week of holiday. But I'm back :)

I just made three commits: 1) Changed every call for _parseiso to _parse_iso. 2) Added tests for _parse_iso() in test_time.py. 3) And then added a few more tests to a few relevant test_x functions in test_time.py.

Sincerely.

hugovk · 2023-07-25T08:54:29Z

Thank you for the update. Please could you check the lint failures? They're mostly type hint things. Let me know if you'd like a hand.

flake8...................................................................Failed
- hook id: flake8
- exit code: 1

tests/test_time.py:102:32: F821 undefined name 'Object'
tests/test_time.py:102:50: F821 undefined name 'Object'
...
mypy.....................................................................Failed
- hook id: mypy
- exit code: 1

src/humanize/time.py:281: error: Incompatible types in assignment (expression has type "Union[date, float, str]", variable has type "Union[datetime, timedelta, float, str]")  [assignment]
src/humanize/time.py:281: error: Argument 1 to "_parse_iso" has incompatible type "Union[datetime, timedelta, float, str]"; expected "Union[date, datetime, float, str]"  [arg-type]
src/humanize/time.py:310: error: Incompatible types in assignment (expression has type "Union[date, float, str]", variable has type "Union[date, datetime]")  [assignment]
src/humanize/time.py:336: error: Incompatible types in assignment (expression has type "Union[date, float, str]", variable has type "Union[date, datetime]")  [assignment]
tests/test_time.py:102: error: Name "Object" is not defined  [name-defined]
Found 5 errors in 2 files (checked 10 source files)

…erifies value is not of type dt.date

velle · 2023-07-25T14:34:41Z

I managed to make all mypy errors disappear. But it was not easy to do this in an elegant way, when _parse_iso takes and returns a number of different types. The code I have pushed is the most elegant solution I could find, but it does contain the following check/cleanup in naturaltime:

_value = _parse_iso(value)
if type(_value) != dt.date:
    # naturaltime is not built to handle dt.date
    value = typing.cast(typing.Union[dt.datetime, dt.timedelta, float, str], _value)

One solution is modifying naturaltime so it would also accept dt.date as value. I looked into that but I wasnt sure what the intended behavior should be and if it made sense for all situations and values.

Another solution is going back to my original approach where the call for fromisoformat() is placed inside each of the naturalX-functions.

def naturaltime(value: dt.datetime | dt.timedelta | float | str)
    if isinstance(value,str):
        try:    value = dt.datetime.fromisoformat(value)
        except: pass

def naturalday(value: dt.date | dt.datetime):
    if isinstance(value,str):
        try:    value = dt.datetime.fromisoformat(value)
        except: pass
        try:    value = dt.date.fromisoformat(value)
        except: pass

def naturaldate(value: dt.date | dt.datetime):
    if isinstance(value,str):
        try:    value = dt.datetime.fromisoformat(value)
        except: pass
        try:    value = dt.date.fromisoformat(value)
        except: pass

Or yet another solution is to create two separate helper functions.

def _try_parse_datetime(value):
    try:
        return dt.datetime.fromisoformat(value)
    finally:
        return value
    
def _try_parse_date(value):
    try:
        return dt.datetime.fromisoformat(value)
    finally:
        return value

def naturaltime(value: dt.datetime | dt.timedelta | float | str)
    value = _try_parse_datetime(value)
    (...)

def naturalday(value: dt.date | dt.datetime):
    value = _try_parse_datetime(value)
    value = _try_parse_date(value)
    (...)

def naturaldate(value: dt.date | dt.datetime):
    value = _try_parse_datetime(value)
    value = _try_parse_date(value)
    (...)

I think I like both of these solutions better than what I pushed. But I'm not sure about anything.

hugovk added the changelog: Added For new features label Jul 9, 2023

hugovk reviewed Jul 9, 2023

View reviewed changes

I realized that humanize does not support datetime.time at all,

6259556

so I removed the ability for _isoparse() to parse into datetime.time. I edited inline documentation and README.md accordingly.

velle and others added 4 commits July 9, 2023 23:01

Update src/humanize/time.py

4ec839e

Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>

every call of _parseiso changed to _parse_iso

9c969cc

test_time.py, added iso8601 tests in a few relevant test_x functions

3517a9d

[pre-commit.ci] auto fixes from pre-commit.com hooks

d5eb5ac

for more information, see https://pre-commit.ci

fixed mypy errors, but not happy with code in time.naturaltime that v…

32014ab

…erifies value is not of type dt.date

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

* time.py, added iso 8601 support #130

* time.py, added iso 8601 support #130

velle commented Jul 9, 2023 •

edited by hugovk

Loading

codecov bot commented Jul 9, 2023 •

edited

Loading

hugovk left a comment

hugovk Jul 9, 2023

velle commented Jul 9, 2023

hugovk commented Jul 9, 2023

velle commented Jul 24, 2023

hugovk commented Jul 25, 2023

velle commented Jul 25, 2023 •

edited by hugovk

Loading


		Relies on the native `fromisoformat` function that exists on date, datetime and time objects.
		```

* time.py, added iso 8601 support #130

Are you sure you want to change the base?

* time.py, added iso 8601 support #130

Conversation

velle commented Jul 9, 2023 • edited by hugovk Loading

codecov bot commented Jul 9, 2023 • edited Loading

Codecov Report

hugovk left a comment

Choose a reason for hiding this comment

hugovk Jul 9, 2023

Choose a reason for hiding this comment

velle commented Jul 9, 2023

hugovk commented Jul 9, 2023

velle commented Jul 24, 2023

hugovk commented Jul 25, 2023

velle commented Jul 25, 2023 • edited by hugovk Loading

velle commented Jul 9, 2023 •

edited by hugovk

Loading

codecov bot commented Jul 9, 2023 •

edited

Loading

velle commented Jul 25, 2023 •

edited by hugovk

Loading