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.

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

Allow custom "now" in naturaldelta and naturaltime #144

Merged
merged 3 commits into from
Oct 3, 2020

Conversation

phijor
Copy link
Contributor

@phijor phijor commented Jul 16, 2020

This allows having a different point in time to which a value is compared.
Useful if one is dealing with datetimes that are not in the local timezone.

Fixes #55.

Changes proposed in this pull request:

  • both naturaldelta and naturaltime now expose an optional now parameter
  • both methods may now be used with datetime objects that are not in the local timezone

@codecov-commenter
Copy link

codecov-commenter commented Jul 16, 2020

Codecov Report

Merging #144 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #144   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            9         9           
  Lines          560       569    +9     
=========================================
+ Hits           560       569    +9     
Flag Coverage Δ
#GHA_Ubuntu 100.00% <100.00%> (ø)
#GHA_Windows 100.00% <100.00%> (ø)
#GHA_macOS 100.00% <100.00%> (ø)

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

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

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55b3e1c...b37dc30. Read the comment docs.

@phijor
Copy link
Contributor Author

phijor commented Jul 16, 2020

So... yeah, black seems to be fighting flake8 here. I cannot put the test cases on multiple lines (black folds them into one), but having them folded into one line by black results in a trailing comma which flake8 does not like. 🤯

@hugovk
Copy link
Collaborator

hugovk commented Jul 18, 2020

Thank you for the PR!

So now could be another timezone, or in the past or future. Would it be better to call it when?

For example now=yesterday or now=next_week might be better as when=yesterday or when=next_week?

So... yeah, black seems to be fighting flake8 here. I cannot put the test cases on multiple lines (black folds them into one), but having them folded into one line by black results in a trailing comma which flake8 does not like. 🤯

Yeah, it's a bit annoying, I think psf/black#1288 is the issue for that.

@phijor
Copy link
Contributor Author

phijor commented Jul 23, 2020

Thank you for the PR!

Thank you for your feedback :) Date, time and timezones are confusing.

So now could be another timezone, or in the past or future. Would it be better to call it when?

For example now=yesterday or now=next_week might be better as when=yesterday or when=next_week?

Calling it when sounds like a good idea. I'll push some changes and resolve conflicts in next few days.

My use case for this is as follows: I have a pair of datetime objects, current_time and starting_time. I cannot control which timezone they are in, but I can assume that it is the same for both of them. Now I want to print (in a human readable way) when starting_time happened, relative to current_time. Something like

>>> (
    f"starting_time is {naturalday(starting_time)}, "
    f"which was {naturaltime(starting_time, when=current_time)}"
)
"starting_time is Jul 12, which was 10 days ago"

Usually it would make sense to just use naturaltime(current_time - starting_time), but the current implementation (date_and_delta) insists on calculating a date relative to _now(), so that won't work. Maybe we can even work around that without having to add a when parameter by refactoring date_and_delta.

@phijor
Copy link
Contributor Author

phijor commented Oct 1, 2020

I was about to update this PR (finally), but it seems like I have borked some tests.

Well, lets see what's wrong.

@phijor
Copy link
Contributor Author

phijor commented Oct 1, 2020

Huh... so it seems like datetime supports subtraction of values in different timezones? Way to subvert my expectations. It just does not like to compare values where tz=None in one operand.

I will squash all these fixup commits before merging.

@hugovk
Copy link
Collaborator

hugovk commented Oct 1, 2020

Ping me when you're ready for re-review :)

This allows having a different point in time to which a value is
compared.  Useful if one is dealing with datetimes that are not
in the local timezone.
@phijor
Copy link
Contributor Author

phijor commented Oct 2, 2020

Ping! @hugovk

Copy link
Collaborator

@hugovk hugovk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Just a little docstring nit.

Examples
Compare two timestamps in a custom local timezone::

from datetime import datetime, timedelta, timezone
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please can you change this example to use import datetime as dt and dt.datetime etc?

Rationale: https://adamj.eu/tech/2019/09/12/how-i-import-pythons-datetime-module/

@hugovk hugovk added the enhancement New feature or request label Oct 3, 2020
@hugovk hugovk merged commit d30c2b6 into jmoiron:master Oct 3, 2020
@hugovk
Copy link
Collaborator

hugovk commented Oct 3, 2020

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The package (wrongly) assumes all times are in the local timezone
3 participants