* time.py, added iso 8601 support #130
Conversation
* 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 Report
@@ 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
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
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.
|
|
||
| Relies on the native `fromisoformat` function that exists on date, datetime and time objects. | ||
| ``` |
There was a problem hiding this comment.
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.
|
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? |
|
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 <[email protected]>
for more information, see https://pre-commit.ci
|
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. |
|
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. |
…erifies value is not of type dt.date
|
I managed to make all mypy errors disappear. But it was not easy to do this in an elegant way, when _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 Another solution is going back to my original approach where the call for 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: passOr 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. |
Fixes #129.