Django

Code

Ticket #3961 (closed: fixed)

Opened 1 year ago

Last modified 6 months ago

Add South African / za localflavor

Reported by: Russell Cloran <russell@rucus.net> Assigned to: nobody
Component: django.contrib.localflavor Version: SVN
Keywords: localflavor, za Cc:
Triage Stage: Ready for checkin Has patch: 1
Needs documentation: 0 Needs tests: 0
Patch needs improvement: 0

Description

The attached patch adds support for South African ID numbers and postal codes.

The patch also creates django.utils.checksums (with only a luhn function for now), as suggested by Malcolm Tredinnick on django-developers

Attachments

za-localflavor.diff (5.3 kB) - added by Russell Cloran <russell@rucus.net> on 04/08/07 03:52:51.
Patch to add za localflavor
ticket_3961__rev_6223.diff (4.6 kB) - added by __hawkeye__ on 09/14/07 16:49:44.
Removed Luhn algorithm implementation, now covered separately in #5475

Change History

04/08/07 03:52:51 changed by Russell Cloran <russell@rucus.net>

  • attachment za-localflavor.diff added.

Patch to add za localflavor

04/08/07 04:29:55 changed by Russell Cloran <russell@rucus.net>

  • needs_better_patch set to 1.
  • summary changed from Add South African / za localflavor to [patch] Add South African / za localflavor.
  • needs_tests changed.
  • needs_docs changed.

I forgot to mention in the original submission ... the Luhn function currently returns False when it gets an input that isn't a string of digits (or an integer) -- it should perhaps raise a ValueError? instead?

(follow-up: ↓ 3 ) 04/08/07 05:54:20 changed by mtredinnick

Yeah, I think the checksum function should raise an error on bad input, rather than pretending to work.

However, could you give the whole checksum function another swing? At the moment, you are converting integers to strings just so you can later convert them back to integers. How about letting integers be integers (and be careful because isintance(True, int) = True in Python, unfortunately) and converting string digits to digits. You could just do long(input_string) and anything that raises a value error is going to be wrong, for example. Check the input for leading zeros (assume leading zeros if it's too short).

I realise this sounds a bit non-specific, but I just get the feeling you're doing this in a slightly awkward fashion.

(in reply to: ↑ 2 ; follow-up: ↓ 8 ) 04/08/07 11:59:05 changed by Russell Cloran <russell@rucus.net>

The function was initially created to accept string input -- I think this is probably the most common use case, since you're likely to store things like credit card numbers as a CharField? in your database. The acceptance of integer types was tacked on, and yes, sits a bit uneasy with me too.

I'm not quite sure I get what you're saying -- should the function accept only integer input? In which case, there is going to be a long() outside the function for most use cases, and instead of a simple generator, the function needs to do %10, /10 work.

04/08/07 21:24:53 changed by adrian

  • component changed from Contrib apps to django.contrib.localflavor.

04/11/07 05:42:41 changed by Simon G. <dev@simon.net.nz>

  • stage changed from Unreviewed to Accepted.

07/30/07 09:39:00 changed by brad@whijo.net

Hi, this patch doesnt seem to be in the SVN tree yet, or am I missing something?

07/30/07 10:45:32 changed by ubernostrum

You'll want to see here for an explanation of what the ticket stages mean. "Accepted" does not mean "patch has been committed" -- that's what "fixed" is for.

(in reply to: ↑ 3 ) 09/14/07 13:56:24 changed by keegan.csmith@gmail.com

Replying to Russell Cloran <russell@rucus.net>:

In the checksum function surely this would suffice for the check:

s = str(s)
if not s.isdigit():
    raise ValueError, "'%s' needs to be a string of digits" % s

I don't see the need to check what type s is, rather just convert it to a string. Or is this against some django conventions?

09/14/07 16:49:44 changed by __hawkeye__

  • attachment ticket_3961__rev_6223.diff added.

Removed Luhn algorithm implementation, now covered separately in #5475

12/01/07 20:00:40 changed by Simon G <dev@simon.net.nz>

  • needs_better_patch deleted.
  • summary changed from [patch] Add South African / za localflavor to Add South African / za localflavor.
  • stage changed from Accepted to Ready for checkin.

12/02/07 12:43:02 changed by mtredinnick

  • status changed from new to closed.
  • resolution set to fixed.

(In [6843]) Fixed #3961 -- Added South African localflavor. Thanks, Russell Cloran.


Add/Change #3961 (Add South African / za localflavor)




Change Properties
Action