Spanish (es) localflavor

[ricardojbarrios@…]

Hi, I've cerated some spanish localflavour code. The tests are in test.py.

I'm working in some extra code, but i'll release it when it's done.

[ricardojbarrios@…]

The system doesn't leave me upload a file, so i've upload it to an external server, in ​http://www.organicadtm.com/localflavour_es.zip

[Simon G. <dev@…>]

Patch from ricardojbarrios@.. that I've just svn diff'ed

[Malcolm Tredinnick]

I don't like the error messages in the identity card number class. If they enter an invalid number, you can't guess at what the right number should be -- the error messages here are assuming it's the check digit that is wrong. That's not a valid assumption, since they may have enterred any one or more of the previous characters incorrectly. It should report that the number is incorrect, only. Then the person entering the data can enter the correct value. Prompting the user with a potentially still incorrect value will increase the chances that they'll just use that wrong one instead.

Before we can check this in:

• Please remove the show_correct option.
• Raise an AssertionError (or just use an "assert" statement) if both nif and cif are not specified, which was one of the "fixme" items in the code.
• Some of the validation statements assume that taking int() of one of the entries will never fail. This isn't true: int('a'), for example. A test that checks that f.clean("elephant") for each of the card types gives a validation error (rather than a Python programming error) would be a good idea.

[ricardojbarrios@…]

The problems with show_correct option can be easily overrided with a more clean error mesage.

The checks are done because previously we have a regex that check that it's one of the correct format. For example, if we use f.clean("elephant") it will raise a u'Please, enter a valid CIF, NIF or NIE.', so, when i use a int i know that it ALWAIS is going to be a int. ;)

I'll add the AssertionError and the new checks today or tomorrow.

[Malcolm Tredinnick]

Unfortunately, the show_correct option cannot be fixed. This is because there are many possible correct options for every incorrect string. Recomputing the check digit is only one possibility. So show_correct really means "show a guess", which is misleading.

[oggie_rob]

I should be able to make those changes & fix the test location.

[Marc Garcia]

contrib/localflavor/es/es_provinces.py (Spain provinces file)

[Marc Garcia]

contrib/localflavor/es/es_regions.py (Spain regions file)

[Marc Garcia]

contrib/localflavor/es/forms.py (Spain NIF validation function)

[Marc Garcia]

I've upload some files for this ticket.
es_provinces.py and es_regions.py should replace existing es_province.py. That's because old file has spanish names, new ones have translatable english names. Also I've changed ISO codes for oficial codes. Regions file didn't exist.

forms.py has a new implementation of NIF validation, that (hopefully) solves old one issues. It should be merged to old forms.py and tested again if after discusing, this one is better for checking in.

oggie_rob I can do that if you want.

[oggie_rob]

New patch with simplified Identity number field & error handling

[oggie_rob]

Using Malcolm's comments and starting with ricardojbarrios' code, and looking through Wikipedia for examples of how this is supposed to work, I think I've come up with reasonable validation methods.

It would help if somebody who actually has a NIE or knows how they work took a look - I wasn't sure whether 7 or 8 digits were the norm.

Unfortunately I wasn't able to post the changes in before garcia_marc submitted patches... oh well. Somewhere in there is an appropriate solution.

[Marc Garcia]

Hi oggie_rob, firs of all sorry for submitting a patch without letting you know that I was working on that, but the fact is that I couldn't find this ticket until I finished my work.

I've checked your code and there is something that IMHO shouldn't be like that, and it's arguments Nif and Cif. Whan you have to validate a NIF field, usually is the customer or the provider in an invoice, and you cannot know in advance if you are going to sell something to a company (that has a CIF code), or to a individual (that has a NIF/NIE code). So I think that you cannot pass an argument to specify it, you have to check it inside the function.

Anyway, I think that you're using some code better than mine, so any kind of merge of both functions would be great. If you want me to assign this ticket to myself, and make those final stepe, please let me know.

[oggie_rob]

Replying to garcia_marc:

Hi oggie_rob, firs of all sorry for submitting a patch without letting you know that I was working on that, but the fact is that I couldn't find this ticket until I finished my work.

I've checked your code and there is something that IMHO shouldn't be like that, and it's arguments Nif and Cif. Whan you have to validate a NIF field, usually is the customer or the provider in an invoice, and you cannot know in advance if you are going to sell something to a company (that has a CIF code), or to a individual (that has a NIF/NIE code). So I think that you cannot pass an argument to specify it, you have to check it inside the function.

Anyway, I think that you're using some code better than mine, so any kind of merge of both functions would be great. If you want me to assign this ticket to myself, and make those final stepe, please let me know.

Hi garcia_marc,
My greatest concern about this is that I'm not sure I got the specs right. I learned about these values by looking at the original patch and by searching online for specifications (mainly from Wikipedia, as I couldn't find many technical references). Also Spanish is not my first language, although I feel that I understood most of the articles well enough. Having said that, it seems to match up with what I had seen online.

Here are some of the things I considered when I was writing this:

1. As opposed to the original code, normalization and validation should be the only functions of the Identity card field. We shouldn't suggest any possible answers, as people really should know their own NIF in full
2. Translating the province names seems unnecessary and potentially a lot of work, so I left them in unicode (rather than ugettext)
3. There are situations where users know if they will expect a NIF/NIS or CIF, and so the author should be able to limit those choices. However, by default both schemes are allowed. Your example of an invoice is one where you need to allow both types, but in another example (e.g. an employee form) you might only expect a NIF/NIS. It doesn't hurt to leave as-is.

If you would like to take it over, that would be fine by me. I think that including the two-digit province code is an improvement, although the two-character lookup is also handy. Something that allows searching of both of those would be ideal.

Thanks,

-rob

[Marc Garcia]

Hi,

I absolutly agree on point 1, of course nobody can guess which character is wrong when checksum fails (may be in more sofisticated algorithms, but not this one). I didn't guess it in my code.

I disagree on point 2, for example, I think that an english speaker want to see "Basque Country" as the region, he wouldn't understant spanish or basque name, alse people from Spain usually say it as "País Vasco", that often doesn't like to basques, that prefer to say it "Euskadi", the basque name. And many other cases like that. In my case I use django in catalan, and it will be annoying for me having spanish names for catalan places in django, after the huge work of translating application. I know that it's further work for translators; I recently translated many swedish, german, holland... city names to Catalan and Spanish, so I know it closely, but I think that it is a must.

About point 3, I just misunderstood that there were an extra feature, I thought that you had to specify it; so I agree with you.

I've assigned this ticket to me, so I want to review code (so as far as I understand I'm more confortable with spanish stuff than you), and if you agree on point 2, I'll replace provinces and regions files.

Anyway I hope having your help on it, so I'm sure that your knowledge on hacking django is better than mine.

Thanks,

Marc

[oggie_rob]

Replying to garcia_marc:

I disagree on point 2, for example, I think that an english speaker want to see "Basque Country" as the region, he wouldn't understant spanish or basque name, alse people from Spain usually say it as "País Vasco", that often doesn't like to basques, that prefer to say it "Euskadi", the basque name. And many other cases like that. In my case I use django in catalan, and it will be annoying for me having spanish names for catalan places in django, after the huge work of translating application. I know that it's further work for translators; I recently translated many swedish, german, holland... city names to Catalan and Spanish, so I know it closely, but I think that it is a must.

Yep, those are good reasons for translating!

I've assigned this ticket to me, so I want to review code (so as far as I understand I'm more confortable with spanish stuff than you), and if you agree on point 2, I'll replace provinces and regions files.

That would be great. Thanks.

-rob

[Marc Garcia]

spanish localflavor files

[Marc Garcia]

Finally I uploaded an updated patch for this ticket.

I've added some new fields (phone number and bank account), and I modified some features of old NIF/CIF functions. Main changes are:

• When NIF or NIE, it does not check number length (most NIFs have 8 digits, but actually it is a sequential number that started on 1).
• When CIF, it is valid both number or letter checksum, so there is no official documentation on it, and different authors have different opinions on which company types can have both checksum types.
• I left just a parameter for allowing only nif or nie values (not cif). Allowing only cif values is meaningless, so if idea is restricting just companies, there is a type that uses owners nif as identity number. I cannot imagine any case that somebody would want to avoid this case of companies in validation.
• Message errors are more detiled now.

This patch is well tested using forms, but there are missing regression tests, so I don't know if there are based on any documentation that I cannot found, or using any script. Could anybody help me on it please? After that and any commiter review it'll be ready for check in.

[Marc Garcia]

spanish localflavor files

[Marc Garcia]

Development is finished and testing is done, so just are missing a commiter review and check it in.

[Malcolm Tredinnick]

(In [6555]) Fixed #4036 -- Added Spanish localflavor. Thanks, ricardojbarrios@… and
oggie_rob.