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

datetime: Add default utc tz for datetime.now(). #570

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ThinkTransit
Copy link
Contributor

This is a reauthor of #561

The datetime module doesn't support naive datetimes and throws an unimplemented error when calling some of the methods such as datetime.now().

Rather than passing a tz object to these methods on each call I feel it would be best to use a default tz (utc) when a tz isn't provided. This would be more CPython compliant for the end user.

This pr adds a default utc timezone in fromtimestamp() if no tz is supplied.

Signed-off-by: Patrick Joy [email protected]

@stinos
Copy link

stinos commented Dec 7, 2022

+1, it would be good to have at least something, like the previous implementation which also had shortcomings but at least had a functioning now()

@ThinkTransit
Copy link
Contributor Author

@jimmo @dpgeorge any chance we can get this one reviewed/merged? I imagine datetime.now() would be one of the more common methods in the datetime library and it would be good to support it and match cpython.

@lorcap
Copy link
Contributor

lorcap commented Sep 18, 2023

I'm not in favor of this merge. From CPython 3.11.5 documentation, datetime.now() is expected to return the current local date and time. MicroPhython doesn't have any notion of local timezone, yet. Current library behaviour is correct, IMHO. User shall provide a valid date and time. Or, if/when discussion 12378 leads to a proper timezone support, datetime.now() results will be different.

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

Successfully merging this pull request may close these issues.

3 participants