Opened 16 years ago
Closed 14 years ago
#12595 closed Bug (fixed)
Localflavor bad arguments handling
| Reported by: | Sayane | Owned by: | Jannis Leidel |
|---|---|---|---|
| Component: | contrib.localflavor | Version: | |
| Severity: | Normal | Keywords: | |
| Cc: | say4ne@…, Mike Fogel | Triage Stage: | Ready for checkin |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
A lot of form fields in django.contrib.localflavor handles arguments to __init__() in a bad way.
def __init__(self, *args, **kwargs):
super(PLPESELField, self).__init__(r'^\d{11}$',
max_length=None, min_length=None, *args, **kwargs)
Every line like first one should be fixed like this:
def __init__(self, max_length=None, min_length=None, *args, **kwargs):
Because if someone will pass max_length or min_length argument to __init__() it will fail (multiple min_length/max_length arguments - additional one will be in kwargs).
It is a big problem, if i'm trying to do something like this:
class PostalCodeField(models.CharField):
def formfield(self, **kwargs):
kwargs.setdefault("form_class", PLPostalCodeField)
return super(self.__class__, self).formfield(**kwargs)
because django.db.models.CharField sets max_length to self.max_length.
I'm attaching patch to fix polish localflavor.
Attachments (3)
Change History (15)
by , 16 years ago
| Attachment: | localflavor_pl_fix.diff added |
|---|
comment:1 by , 16 years ago
| Cc: | added |
|---|
comment:2 by , 16 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:3 by , 16 years ago
| Owner: | changed from to |
|---|
comment:4 by , 16 years ago
| milestone: | 1.2 |
|---|---|
| Version: | SVN |
comment:5 by , 15 years ago
| Cc: | added |
|---|
comment:6 by , 15 years ago
| Owner: | removed |
|---|
comment:7 by , 15 years ago
| Severity: | → Normal |
|---|---|
| Type: | → Bug |
comment:8 by , 15 years ago
| Needs tests: | set |
|---|
comment:9 by , 15 years ago
| Easy pickings: | unset |
|---|---|
| Patch needs improvement: | set |
by , 15 years ago
| Attachment: | 12595-1-wip.diff added |
|---|
by , 14 years ago
| Attachment: | 12595-2.diff added |
|---|
comment:10 by , 14 years ago
| Needs tests: | unset |
|---|---|
| Patch needs improvement: | unset |
OK, fixed now in all localflavors. Tested in utils.assertFieldOutput.
comment:11 by , 14 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
I basically see what's the problem and what to do. Now there is a lot of similar changes to make, and I would like to know if I'm going in the right direction before giving more time to this.
See my patch, and tell me if it's OK (and good practice) to pass max_length and min_length to super as args (vs kwargs).
For tests, should I add field_kwargs to all tests, or add a second assert with field_kwargs to all tests, or simply add field_kwargs once per test case?