Shouldn't custom fields in ModelForms pick up model Field options by default?

[Kevin Henry]

Take a simple Model and ModelForm like this:

class Author(models.Model):
name = models.CharField(max_length=80, blank=True, default="Joe",
help_text="A name")

class AuthorForm(forms.ModelForm):

class Meta:
model = Author

The resulting form will reflect the desired help text, default, and
blank options.

But if you decide to customize the widget, for example:

class AuthorForm(forms.ModelForm):
name = forms.CharField(widget=forms.TextInput(attrs={'size':80}))

class Meta:
model = Author

the form field no longer reflects these model field options (that is,
there is no help text, no default value, and you can't save it blank).

This seems like undesirable behavior. At the very least it's
surprising, and should probably be documented here:
http://docs.djangoproject.com/en/dev/topics/forms/modelforms/#overriding-the-default-field-types.

I'm fairly new to Django, so unfortunately I'm not sure of the right
way to do this. One workaround is to set the fields manually, like:

name = forms.CharField(widget=forms.TextInput(attrs={'size':80}),
required=False, initial="Joe", help_text="A name")

but that's an obvious violation of DRY. Another workaround, which
takes advantage of the fact that the options are getting properly set
in the call to the model field's formfield() method, is this:

name = Author._meta.get_field("name").formfield
(form_class=forms.CharField, widget=forms.TextInput(attrs={'size':
80}))

but that's not ideal either, for obvious reasons.

Any thoughts?

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---

Re: Shouldn't custom fields in ModelForms pick up model Field options by default?

[Kevin Henry]

> Well, surprise is in the eye of the participant. As noted above, it
> would be surprising to a lot of us if it did pick up the attributes
> because it removes control from the user. The current behaviour isn't
> illogical.

Oh, I wasn't suggesting that the user shouldn't be able to override
those values, I was only talking about what the default behavior
should be if the user doesn't specify anything. If the user
instantiates a form field, and doesn't specify what 'required' should
be (and the field itself doesn't set this value), it just seems far
more natural to pick up the value the user already specified in the
model, rather than arbitrarily choosing 'True'.

Part of the reason I think this represents a common use case is that
these attributes seem fairly orthogonal to the purposes of validating/
rendering that fields and widgets are used for. That is, I wouldn't
imagine that it's common that your choice of which field or widget to
use to represent a model field will affect whether or not that field
is required, say. Of course, my experience is limited.

I don't know enough yet to suggest how this would be implemented, I
just wanted to throw the idea out there. But unless there's a
groundswell of support for my position, I guess I'll just file a
documentation ticket.

Cheers,
Kevin

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---

Re: Shouldn't custom fields in ModelForms pick up model Field options by default?

[Kevin Henry]

In terms of backward compatibility, perhaps there could be a new
keyword argument to Field, something like with_model_defaults, that
the user would use to indicate a desire to get the attributes from the
Model. How would this work? Maybe something like this (this is just a
sketch): Field.__init__() works the same way, except that it also
stores the value of the keyword arguments (with_model_defaults, as
well as the others, since the user should be able to override
individual settings). Then ModelForm's metaclass will call a method on
the field, passing in the field name. The field can then extract the
default values from the Model and set itself up appropriately.

Too much trickery? Before looking into how Django works I would have
definitely said yes, but now my standards have changed. :) (That's not
a criticism, I'm very happy with the trickery Django implies to make
life easier for the user.)


> There's also a ticket waiting for check-in implementing this common
> usecase:http://code.djangoproject.com/ticket/9223

Thanks, the accompanying discussion is very interesting and on point.
But as someone there points out, why stop at widgets? The discussion
strengthens my intuition that widgets, fields, and the attributes I'm
discussing here are fairly orthogonal concepts, and the current all-or-
nothing declarative syntax couples them too tightly. Both your
solution and the (much less thought-out) one I just proposed feel too
small to me, but I'm not sure of a better solution.

Cheers,
Kevin

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---

Re: Shouldn't custom fields in ModelForms pick up model Field options by default?

[Kevin Henry]

> Please file a ticket for the documentation fix it would be greatly  
> appreciated. :-)

Will do.

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---

Re: Shouldn't custom fields in ModelForms pick up model Field options by default?

[Kevin Henry]

> > Thanks, the accompanying discussion is very interesting and on point.
> > But as someone there points out, why stop at widgets?
>
> Because customizing widgets is a common request, unlike other attributes.

But customizing fields is quite common, no? I agree that it's rare
that you will want to override required, help_text, etc.—in fact,
that's exactly my point. The current syntax forces you to do so when
customizing the form just to get back to the behavior already
specified in the model. Your solution solves this problem for the
common case of customizing the widget, but the issue is just as
applicable for the common case of customizing the field.

But at this point, I'll come up with a more concrete proposal before
carrying on.

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en
-~--~~~~--~~--~--~---

Re: Proposal: Drop dependency on pytz in favor of zoneinfo

[Kevin Henry]

Thanks to Paul for this proposal, and for working to put proper timezone 
usage into Python itself. PEP 495 and 615 definitely make Python better, 
and it seems inevitable that everyone will sooner or later switch over. I'm 
all for getting this process going in Django.

I have doubts over whether it's a good idea to use the shim, though. 
Briefly: 1) The shim is not backward compatible with pytz; 2) To avoid the 
subtle bugs resulting from that, the developer must review pretty much the 
whole scope of their datetime usage when they adopt the shim; therefore 3) 
it will probably be easier on everyone (both developers and Django itself) 
to simply switch directly to zoneinfo.

1) I'm referring to the change in semantics around datetime arithmetic (see 
here 

 
for the description in the shim documentation, and here 
 for a simple demonstration). 
Basically, if you're saying anything like "give me the datetime one day 
after this one", the value (both the wall-clock time and the actual 
point-in-time) could change once you stop using pytz.

2) Because of the above, the developer can't simply drop in the shim as a 
replacement for pytz. They need to review their codebase to see if they're 
doing datetime arithmetic anywhere. If they are, and if the change could 
affect the resulting values, they need to think about whether they want to 
use the old value or the new value. If they want the old value, they need 
to add some more code to recreate it. If they want the new value, they need 
to think about how to reconcile that change with previous behavior (and 
previous values stored in the database).

3) That's the hardest part of the transition, I think, so I'm not sure it's 
actually helpful to delay the rest. Removing calls to localize() and 
normalize() is comparatively simple. The difference between is_dst and fold 
does require some thought, but it's pretty much the same thought you need 
to do arithmetic right, and developers will benefit from making those 
changes at the same time.

In essence, the shim creates another, third way of handling datetimes, 
neither backwards compatible with pytz (due to the change in semantics) nor 
forward compatible with zoneinfo (due to the use of normalize(), etc.). And 
developers might end up in this twilight zone for years given typical 
Django deprecation cycles.

So my thought is simply to not use the shim; developers should either be 
doing things the pytz way or the Python (zoneinfo) way. I don't have strong 
feelings about how that should happen (e.g. a feature flag), or when it 
should happen.

Cheers,
Kevin

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/c3a3a788-2300-4361-ab06-0d89523424ecn%40googlegroups.com.

Re: Proposal: Drop dependency on pytz in favor of zoneinfo

[Kevin Henry]

> The point about the arithmetic semantics is a good one. I'm curious to 
> know how often this is actually a problem.
>
That’s an interesting question. I think that doing localized datetime 
arithmetic in Django is idiomatic, if not necessarily common. Let’s say we 
have a calendar application that allows users to create events and set 
reminders. A typical way to handle an event creation request would be to 
activate() the user’s timezone and then read a localized datetime from the 
Form. Now, to create reminders, you add a timedelta of a day, a month, etc. 
and then store that result in the database. This datetime could be 
different under the new semantics.


2. Add a feature flag that allows switching directly to zoneinfo that will 
> eventually default to True, and replace pytz with a shim *around pytz* 
> that raises warnings whenever you use something pytz-specific. That will 
> give people time to opt in to using zoneinfo with, hopefully, zero changes 
> in the intermediate. (Unfortunately, though, it doesn't allow for any 
> incremental migration like pytz_deprecation_shim does).
>

That is approximately what I was thinking. For the sake of example, I’ll 
present an opt-in approach modeled after the New Middleware transition 
(though I haven’t given the opt-in mechanism much thought):

- In version X, Django deprecates usage of pytz and introduces the TIMEZONE 
setting. It has the same meaning as TIME_ZONE, but the use of it signals 
that the user is opting into zoneinfo. If it’s set, Django does not use or 
assume pytz.
- If it’s not set, Django uses pytz and gives the user deprecation 
warnings. Those could come from a system check; or when Django datetime 
utilities are used; or via a shim around timezone objects; or something 
else.
- In version Y, pytz and TIME_ZONE usage is removed.

As you say, the main difference from your proposal is that there’s no way 
to mix pytz and zoneinfo. Trying to allow that would slow down and 
complicate the transition, and I'm just not seeing much of a benefit to 
outweigh that. (Just one perspective, of course...)

Cheers,
Kevin

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers  (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to django-developers+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-developers/9072bb07-9455-456e-b783-2834a84eab4an%40googlegroups.com.

Re: Proposal: Drop dependency on pytz in favor of zoneinfo

[Kevin Henry]

I think that the simplest approach—the one that would result in the least 
amount of total work for both Django and its users—would be to adopt Nick's 
suggestion and just switch to zoneinfo in 4.0. The problem is that it's 
very hard to square that with Django's stability policy: "We’ll only break 
backwards compatibility of these APIs without a deprecation process if a 
bug or security hole makes it completely unavoidable."

If we're going to follow the deprecation process, then there needs to be 
some overlap where both ways of doing things are possible. The shims 
package is a promising approach, but the fact that it's not actually 
backwards compatible with pytz is a serious problem. Adopting it directly 
as Carlton proposes also seems to violate the stability policy, albeit in a 
less severe way.

Before looking at alternatives, I wonder if we can just change the shims 
package to make it fully backwards compatible? Right now the shims version 
of normalize() 

 
is essentially a noop. Paul, couldn't it actually attempt to adjust the 
time the way pytz does? Perhaps by wrapping pytz itself, and calling its 
normalize() from the corresponding pytz timezone; or by simply replicating 
its time-changing logic? Apologies if that's a naive question.

Cheers,
Kevin

On Thursday, October 8, 2020 at 11:35:21 PM UTC-7 smi...@gmail.com wrote:

> Hi All,
>
> While I understand the desire to have an early opt-in for some I think the 
> important question here is the deprecation warnings. The recent URL() 
> change showed that no matter how long there is a new way some?/many? folk 
> won't change until they need to. 
>
> Nick -- if we introduced a breaking change in 4.0, would that not have the 
> same impact on folk upgrading to 4.2LTS from 3.2LTS as that which Carlton 
> is concerned about (3.2 from 2.2), albeit a few years further into the 
> future. 
>
>
> David
>
> On Thursday, 8 October 2020 at 09:08:50 UTC+1 jure.er...@gmail.com wrote:
>
>> I would definitely be in favor of an opt-in: it would give developers 
>> time to move to the new system at their convenience.
>>
>> Example: we're about to try and tackle the TZ issue in our apps and we 
>> want to do it "globally" with one definitive solution. I'd much rather do 
>> it on a library that is currently favoured, but not yet default than on a 
>> deprecated one, even if it's not yet officially deprecated. We do have some 
>> "import pytz", but currently they are few. Once we have a proper approach 
>> to handling timezone stuff, there's likely going to be more of them... or 
>> less, depending on the solution ;-)
>>
>> LP,
>> Jure
>> On 7. 10. 20 17:25, Paul Ganssle wrote:
>>
>> This sounds like a reasonable timeline to me. I think the breakage will 
>> be relatively small because I suspect many end-users don't really even know 
>> to use `normalize` in the first place, and when introducing the shim into a 
>> fundamental library at work I did not get a huge number of breakages, but I 
>> am still convinced that it is reasonably categorized as a breaking change.
>>
>> I do think that there's one additional stage that we need to add here 
>> (and we chatted about this on twitter a bit), which is a stage that is 
>> fully backwards compatible where Django supports using non-pytz zones for 
>> users who bring their own time zone. I suspect that will help ease any 
>> breaking pain between 3.2 and 4.0, because no one would be forced to make 
>> any changes, but end users could proactively migrate to zoneinfo for a 
>> smoother transition.
>>
>> I think most of what needs to be done is already in my original PR, it 
>> just needs a little conditional logic to handle pytz as well as the shim.
>>
>> I am not sure how you feel about feature flags, but as a "nice to have", 
>> I imagine it would also be possible to add a feature flag that opts you in 
>> to `zoneinfo` as time zone provider even in 3.2, so that people can jump 
>> straight to the 5.0 behavior if they are ready for it.
>>
>> I should be able to devote some time to at least the first part — making 
>> Django compatible with zoneinfo even if not actively using it — but likely 
>> not for a few weeks at minimum. If anyone wants to jump on either of these 
>> ahead of me I don't mind at all and feel free to ping me for review.
>>
>> Best,
>> Paul
>> On 10/7/20 10:48 AM, Carlton Gibson wrote:
>>
>> Hi Paul.  
>>
>> Thanks for the input here, and for your patience 
>>
>> > I am fairly certain this is going to be a tricky migration and will 
>> inevitably come with *some* user pain. I don't think this will be Python 
>> 2 → 3 style pain, but some users who have been doing the "right thing" with 
>> pytz will need to make changes to their code in the long run, which is 
>> unfortunate.
>>
>> Looking at all the docs, your migration guide on pytz_deprecation_shim, 
>> the exa

Re: Proposal: Drop dependency on pytz in favor of zoneinfo

[Kevin Henry]

 and tells people 
> to turn on the "use zoneinfo" feature flag. It has the upside of being 
> fully backwards-compatible, but the downside of prolonging dependence on 
> pytz.
>
> Another option is to modify the shims so that `normalize` always raises an 
> exception instead of a warning (or maybe it raises an exception for 
> anything except UTC and fixed offsets). In that case, version 4.0 will 
> *mostly* just work and start raising deprecation warnings, but there will 
> be a hard breakage for anyone who would be negatively affected by the 
> change in semantics. This *would* still leave a possible problem in the 
> other direction, though:
>
> >>> from datetime import datetime, timedelta
> >>> from zoneinfo import ZoneInfo
> >>> import pytz
> >>> NYC_p = pytz.timezone("America/New_York")
> >>> NYC = ZoneInfo("America/New_York")
>
> >>> dtp_0 = NYC_p.localize(datetime(2020, 10, 31, 12))
> >>> dtp_1 = NYC_p.localize(datetime(2020, 11, 1, 12))
> >>> (dtp_1 - dtp_0 ) / timedelta(hours=1)
> 25.0
>
> >>> dtz_0 = datetime(2020, 10, 31, 12, tzinfo=NYC)
> >>> dtz_1 = datetime(2020, 11, 1, 12, tzinfo=NYC)
> >>> (dtz_1 - dtz_0) / timedelta(hours=1)
> 24.0
>
> This occurs because localized pytz zones are different tzinfo objects, and 
> as such comparisons and subtraction use inter-zone semantics. Of course, 
> you'll have this same problem even with a "hard break", since unlike 
> invocation of `normalize` and `localize`, subtraction operations will 
> succeed if you swap out the attached tzinfo for a zoneinfo tzinfo.
>
> If we go with any variation of using shim-around-zoneinfo like 
> pytz-deprecation-shim, I'd say those shims need to be introduced as a 
> breaking change in Django 4.0. If we go with shim-around-pytz, I think that 
> can safely be introduced in 3.2 (though that would *require* 
> simultaneously adding support for using zoneinfo, and even then it might 
> *mostly* force people to either do the migration in a single huge step or 
> to involve some wrapper functions for handling the period of time where the 
> time zone type is not consistent throughout the application).
>
> Best,
> Paul
> On 10/9/20 9:31 AM, Kevin Henry wrote:
>
> I think that the simplest approach—the one that would result in the least 
> amount of total work for both Django and its users—would be to adopt Nick's 
> suggestion and just switch to zoneinfo in 4.0. The problem is that it's 
> very hard to square that with Django's stability policy: "We’ll only break 
> backwards compatibility of these APIs without a deprecation process if a 
> bug or security hole makes it completely unavoidable."
>
> If we're going to follow the deprecation process, then there needs to be 
> some overlap where both ways of doing things are possible. The shims 
> package is a promising approach, but the fact that it's not actually 
> backwards compatible with pytz is a serious problem. Adopting it directly 
> as Carlton proposes also seems to violate the stability policy, albeit in a 
> less severe way.
>
>
> Kevin
>
> On Thursday, October 8, 2020 at 11:35:21 PM UTC-7 smi...@gmail.com wrote:
>
>> Hi All, 
>>
>> While I understand the desire to have an early opt-in for some I think 
>> the important question here is the deprecation warnings. The recent URL() 
>> change showed that no matter how long there is a new way some?/many? folk 
>> won't change until they need to. 
>>
>> Nick -- if we introduced a breaking change in 4.0, would that not have 
>> the same impact on folk upgrading to 4.2LTS from 3.2LTS as that which 
>> Carlton is concerned about (3.2 from 2.2), albeit a few years further into 
>> the future. 
>>
>>
>> David
>>
>> On Thursday, 8 October 2020 at 09:08:50 UTC+1 jure.er...@gmail.com wrote:
>>
>>> I would definitely be in favor of an opt-in: it would give developers 
>>> time to move to the new system at their convenience.
>>>
>>> Example: we're about to try and tackle the TZ issue in our apps and we 
>>> want to do it "globally" with one definitive solution. I'd much rather do 
>>> it on a library that is currently favoured, but not yet default than on a 
>>> deprecated one, even if it's not yet officially deprecated. We do have some 
>>> "import pytz", but currently they are few. Once we have a proper approach 
>>> to handling timezone stuff, there's likely going to be more of them... or 
>>> less, depending on the solution