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

Fixed #24281 -- Improved docs for timezone handling for auto_now and auto_now_add #4364

Conversation

chrisjluc
Copy link
Contributor

and convert to a date format for display only.

``auto_now`` and ``auto_now_add`` become ambiguous as well.
It's recommended to set a ``default`` value that does what you expect regarding timezones.
Copy link
Member

Choose a reason for hiding this comment

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

Overriding save() provides a better implementation of what users of auto_now(_add) expect.

Copy link
Member

Choose a reason for hiding this comment

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

Can you explain this comment? For auto now, only overriding save works. For auto now add, I'm not aware of any reason to prefer it over a default.

Copy link
Member

Choose a reason for hiding this comment

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

You're right: default works as a replacement for auto_now_add — provided it's serializable by migrations.

I still find it better to document "override save()" than to get into a discussion of what is serializable by migrations and why default can replace auto_now_add but not auto_now.

@carljm
Copy link
Member

carljm commented Mar 23, 2015

@chrisjluc Here's the wording that I would propose adding to the DateField docs:

.. note::
    The ``auto_now`` and ``auto_now_add`` options will always use the server operating
    system's local date at the moment of creation or update, regardless of the
    :setting:`TIME_ZONE` setting or the currently active timezone. If you need
    something different, you may want to consider simply using your own callable
    default or overriding `save()` instead of using ``auto_now`` or ``auto_now_add``;
    or using a `DateTimeField` instead of a `DateField` and deciding how to handle
    the conversion from datetime to date at display time.

@aaugustin does that seem like reasonable advice to you?

@aaugustin
Copy link
Member

@carljm That's almost correct. These options will always use the default time zone, which is defined by settings.TIME_ZONE. Only when settings.TIME_ZONE is None does Django use the system's time zone. The safest bet is to link to the documentation of the default time zone :-)

Otherwise, sounds great to me.

@carljm
Copy link
Member

carljm commented Mar 23, 2015

Hmm, I thought I'd tested the other day and observed that settings.TIME_ZONE seemed to have no effect on datetime.date.today(). But I must have been doing something wrong in my test.

@carljm
Copy link
Member

carljm commented Mar 23, 2015

Yep, I was wrong; datetime.date.today() does respect TIME_ZONE. Not sure what I was doing the other day. So here's a revised proposal:

.. note::
    The ``auto_now`` and ``auto_now_add`` options will always use the date in the
    `default timezone`_ at the moment of creation or update. If you need
    something different, you may want to consider simply using your own callable
    default or overriding `save()` instead of using ``auto_now`` or ``auto_now_add``;
    or using a `DateTimeField` instead of a `DateField` and deciding how to handle
    the conversion from datetime to date at display time.

As Aymeric says, "default timezone" should link to https://docs.djangoproject.com/en/1.7/topics/i18n/timezones/#default-time-zone-and-current-time-zone - I didn't look up the Sphinx reference needed for that.

@carljm
Copy link
Member

carljm commented Mar 29, 2015

This looks good to me. @chrisjluc can you squash it back down to one commit?

@chrisjluc chrisjluc force-pushed the 24281_improve_docs_timezone_handling branch from 374e77c to dec896d Compare March 29, 2015 19:51
@chrisjluc
Copy link
Contributor Author

@carljm the commits have been squashed.

@MarkusH
Copy link
Member

MarkusH commented Mar 29, 2015

Fixed in 8119876. Thanks.

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.

5 participants