Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Accept ISO 8601 string, in addition to datetime/date/time object #129

Open
velle opened this issue Jul 8, 2023 · 5 comments · May be fixed by #130
Open

Accept ISO 8601 string, in addition to datetime/date/time object #129

velle opened this issue Jul 8, 2023 · 5 comments · May be fixed by #130

Comments

@velle
Copy link

velle commented Jul 8, 2023 •
edited by hugovk
Loading

Motivation:

I believe that the humanize functions are often used indrectly from template engines. In my case I use Jinja2 templating engine and the jinja2_humanize_extension, which relies on the humanize package.

{{ value | humanize_naturaldate }}

In many cases the underlying data (either a database, json-file or csv-file) stores/returns the datetime (or date or time) as a string, formatted in ISO8601 format. But in order to make humanize_naturaldate accept this, I need to first parse it.

{{ value | fromisoformat | humanize_naturaldate }}

The fromisoformat then needs to be set as a filter in the local or global jinja environment.

It would be great if humanize accepted these ISO8661 strings itself.

How to implement:

As far as I can tell, it would be very simple to implement and very simple to maintain. Every function callable by the client, that takes a datetime should start with:

if type(value) == str:
    try:
        value = datetime.datetime.fromisoformat(value)
    else:
        value = str

Similarly for date and time objects.

I will admit that I have only studied the code cursorily, and I might have missed some reasons why this is more complex than I currently believe it is.
@hugovk
Copy link
Member

hugovk commented Jul 8, 2023

Would you like to investigate putting a PR together?

That initial bit of transformantion code could be an internal (name starting with _) function.

@velle
Copy link
Author

velle commented Jul 8, 2023

I would gladly attempt that. However this will be my first contribution to an open source project or any other git managed project, and I yet have to learn the technicalities of forking, branching, doing the pull request, etc. But I started reading about that just now :)

@hugovk
Copy link
Member

hugovk commented Jul 8, 2023

Exciting! Don't hesitate to ask if you've any questions.

@velle
Copy link
Author

velle commented Jul 8, 2023 •
edited
Loading

I managed to fork and create a branch "iso8601". I managed to push that to my fork (I know my understanding and terminology is vague...). You can see it here: https://github.com/velle/humanize/tree/iso8601.

Should we evaluate the code now, or should I somehow push that branch to python-humanize/humanize before we proceed? And how do I do that?

I think I might actually have written code that works. And documentation :) but...

I don't know how to run tox in any other way than just typing "tox". That gives me errors from both mypy and lint. I managed to remove some of the mypy errors, but not all, and I had a feeling I was doing it wrong. I have not figured out how to see the output of lint, so I have not fixed any of those.

@hugovk
Copy link
Member

hugovk commented Jul 9, 2023

Great!

Let's create a PR and evaluate it there. It will also run the CI and we can check what's failing, and iterate from there.

  1. go to https://github.com/velle/humanize/tree/iso8601
  2. click "1 commit ahead"
  3. click "create pull request"

I don't know how to run tox in any other way than just typing "tox". That gives me errors from both mypy and lint. I managed to remove some of the mypy errors, but not all, and I had a feeling I was doing it wrong. I have not figured out how to see the output of lint, so I have not fixed any of those.

Yep, "tox" is the usual way to run it. It will also run those checks on CI and we can see what needs changing there. tox also runs lint.

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.

@hugovk hugovk linked a pull request Jul 9, 2023 that will close this issue
Open
@hugovk hugovk changed the title Accept iso 8601 string, in addition to datetime/date/time object Accept ISO 8601 string, in addition to datetime/date/time object Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

* time.py, added iso 8601 support velle/humanize
2 participants
@hugovk @velle