search

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
489 stars 204 forks source link

Addition of rules for Pakistani CNIC/SNIC (Computerized/Smart National Identity Card) #306

Closed QuantumNovice closed 1 year ago

QuantumNovice commented 1 year ago

304

arthurdejong commented 1 year ago

Hi @QuantumNovice,

Thanks for providing the PR.

Can you clarify the copyright/author of this change. The file currently contains

 # Author: Quantum Novice (Syed Haseeb Shah)

Is it appropriate to change this into the following?

# Copyright (C) 2022 Syed Haseeb Shah

Also, the mentioned Wikipedia page suggests that the unique number is called the "NIC number" and CNIC and SNIC are the names of the cards. Can you confirm the name of the number?

Do you happen to have a couple more test numbers that can be used to validate the implementation?

Btw, the implementation provided is a good starting point but I think I'll re-work some bits to have them be more in line with other implementations.

QuantumNovice commented 1 year ago

Hi @QuantumNovice,

Thanks for providing the PR.

Can you clarify the copyright/author of this change. The file currently contains

 # Author: Quantum Novice (Syed Haseeb Shah)

Is it appropriate to change this into the following?

# Copyright (C) 2022 Syed Haseeb Shah

I will remove that author description if that is now allowed. The code is not my copyright and should be distributed/redistributed or modified in accordance with license of the repository. The code is not my copyright and should be freely distributed in the spirit of OpenSource.

Also, the mentioned Wikipedia page suggests that the unique number is called the "NIC number" and CNIC and SNIC are the names of the cards. Can you confirm the name of the number? The official website of the ID Card (NADRA) refers to it as NIC but almost all government website refer to it by the abbreviation CNIC. NIC and SNIC are rarely used. Some government calling it CNIC are HEC, PEC, etc.

Do you happen to have a couple more test numbers that can be used to validate the implementation?

Yes, of course. What should be the number of test cases?

Btw, the implementation provided is a good starting point but I think I'll re-work some bits to have them be more in line with other implementations.

Perfect, guess I'll become familiar with the code base in the process.

Thank you for the review.

Regarding the flake8 tests. I'll remove build errors.

arthurdejong commented 1 year ago

I will remove that author description if that is now allowed. The code is not my copyright and should be distributed/redistributed or modified in accordance with license of the repository. The code is not my copyright and should be freely distributed in the spirit of OpenSource.

If you wrote the code, you are the author and copyright holder. If you wrote the code in time of your employer, the company is generally the copyright holder but you will still be the author. The suggested change is just for clarification (see this section of the GPL for more info). As long as it is licensed under the LGPL I can integrate it into python-stdnum.

Yes, of course. What should be the number of test cases?

Ideally about a dozen or so. For example see https://github.com/arthurdejong/python-stdnum/blob/master/tests/test_by_unp.doctest#L67-L97

arthurdejong commented 1 year ago

Hi @QuantumNovice,

I've merged an implementation of the Pakistani ID card number as fa62ea3. It is somewhat based on the implementation you provided. I could not find any explanation of the check digit validation and I also couldn't find an online validation service that would allow me to check if there is a check digit (or if it is just a digit for gender as some sources seem to suggest).

If you have any more information (e.g. examples of known valid numbers) I'd be happy to improve the implementation.

Thanks for providing your implementation!