arthurdejong / python-stdnum

A Python library to provide functions to handle, parse and validate standard numbers.
https://arthurdejong.org/python-stdnum/
GNU Lesser General Public License v2.1
498 stars 206 forks source link

Added support for AIC codes #193

Closed FabrizioMontanari closed 4 years ago

FabrizioMontanari commented 4 years ago

AIC codes are used to identify drugs allowed to be sold in Italy. Codes are assigned by AIFA, or "Agenzia Italiana del Farmaco" (Italian Medicines Agency), the italian authority responsible for drugs regulation in Italy. The italian Government defined how these codes are structured in an official bill available here, valid since the 30th of may 2014.

arthurdejong commented 4 years ago

Hi @FabrizioMontanari

Thanks for your pull request. Since you also seem to be the original author of pyAIC are you OK with licensing this under the LGPL so it can be included in python-stdnum?

I'll have a look. There will probably be some adaptations to follow the rest of the coding style (e.g. taking advantage of the clean() function, adding a compact() function, etc. The validate() function is also expected to raise an exception when the validation fails (the is_valid() function is expected to only return a boolean).

arthurdejong commented 4 years ago

Also, is it true that the BASE10 representation is the canonical form and the BASE32 version is just for encoding into a bar code? The reason I'm asking is that most other validate() implementations return the canonical form of the number or code.

From there we could either have conversion functions to and from the BASE32 form or automatically convert the BASE32 form to the BASE10 representation.

FabrizioMontanari commented 4 years ago

Hi @arthurdejong , I'm the original author of pyAIC. For me is ok to release this pull request under LGPL.

I will start refactoring code and tests to better comply with the existing interface. I will take a look at stdnum.ultils, stdnum.luhn and stdnum.exceptions to improve validate() and is_valid(), and also to implement a compact(). Is there any other standard function that must be implemented?

Regarding the relationship between BASE10 and Base32, BASE10 should be the canonical form, while BASE32 is for barcodes, as you wrote. The official paper linked above defines a checksum only for BASE10 representation, so conversion function are needed. How should these conversion function be named?

arthurdejong commented 4 years ago

Apart from the functions you mentioned no others are really required. I don't think a format() function would add much in this case.

Since BASE10 should be the normal form, I would say the module could implement from_base32() and to_base32() functions for the conversions. The validate() and compact() functions should then probably either only accept the BASE10 form or automatically convert a BASE32 form to BASE10. If there is a need to distinguish between the formats a utility function for that could be made.

Btw, the check digit calculation can be rewritten into something like:

def calc_check_digit(number):
    """Calculate the check digit for the BASE10 AIC code."""
    weights = (1, 2, 1, 2, 1, 2, 1, 2)
    return str(sum((x // 10) + (x % 10)
               for x in (w * int(n) for w, n in zip(weights, number))) % 10)

Thanks.

FabrizioMontanari commented 4 years ago

Updated functions and tests. I also added the licence, can you check if is correct?

arthurdejong commented 4 years ago

Hi @FabrizioMontanari,

I've merged your PR with some modifications as 8433821. Thanks for providing the code!