Add the module for Ecuadorian Identification Card (CI - Cedula de identi...

[jonathanf]

...dad) and Fiscal Numbers (RUC - Registro Unico de Contribuyentes)

Dear Arthur, check out my contribution for the proyect.

Best regards,
Jonathan

[arthurdejong]

Thanks. A few comments:

• the tests current fail (the examples in the docstring do not match the expected output)
  (you should be able to run the tests with nosetests)
• incorrect formatting of numbers does not raise the correct exceptions:
  ci.validate('123A567890') raises ValueError instead of InvalidFormat
• in stdnum.ec.ci it is probably clearer to factor out a separate function and reduce it to:

  def calc_check_digit(number):
      """Calculate the check digit. Any check digits present in the number
      are ignored"""
      total = sum((int(number[i]) * (2 - x % 2)) % 9 for i in range(9))
      return str((10 - total) % 10)

(the ruc module can probably also be simplified a bit to use sum() like above instead of for loops)

• in stdnum.ec.ruc the module docstring says "third digit is a letter" but this isn't reflected in the code

If you run nosetests like above you also get a coverage report that shows lines that are not covered by tests.

Can you make the corrections above? Thanks for your contribution!

[jonathanf]

Of course, the next week you will have this.

Best regards,
Jonathan

Enviado desde Samsung Mobile

-------- Mensaje original --------

De: Arthur de Jong notifications@github.com

Fecha:09/08/2014 8:18 AM (GMT-05:00)

A: arthurdejong/python-stdnum python-stdnum@noreply.github.com

CC: Jonathan Finlay jfinlay@riseup.net

Asunto: Re: [python-stdnum] Add the module for Ecuadorian Identification Card (CI - Cedula de identi... (#12)

Thanks. A few comments:

the tests current fail (the examples in the docstring do not match the expected output) (you should be able to run the tests with nosetests)
incorrect formatting of numbers does not raise the correct exceptions: ci.validate('123A567890') raises ValueError instead of InvalidFormat
in stdnum.ec.ci it is probably clearer to factor out a separate function and reduce it to:
def calc_check_digit(number):
"""Calculate the check digit. Any check digits present in the number
are ignored"""
total = sum((int(number[i]) * (2 - x % 2)) % 9 for i in range(9))
return str((10 - total) % 10)
(the ruc module can probably also be simplified a bit to use sum() like above instead of for loops)

in stdnum.ec.ruc the module docstring says "third digit is a letter" but this isn't reflected in the code
If you run nosetests like above you also get a coverage report that shows lines that are not covered by tests.

Can you make the corrections above? Thanks for your contribution!

—
Reply to this email directly or view it on GitHub.

[jonathanf]

Hey, i hope there is done. Tell me if something is not right!

[arthurdejong]

Thanks for the update. I have merged your changes with some modifications.

I can't read Spanish so please review the changes and point out any errors in the code. I've had a look at https://github.com/diaspar/validacion-cedula-ruc-ecuador and a few other documents I found online and understand that a natural RUC (third digit 0..5) is a CI followed by an establishment number. This means that the validation of a natural RUC can be delegated to the CI module.

I've also added tests for more numbers that I found online to ensure that various possible errors in check digit calculations should be covered now (this brought to light a issue in your calculation for natural the RUC).