Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add predefined_recognizer: IN_AADHAAR #1256

Merged
merged 21 commits into from
Jan 30, 2024
Merged

Add predefined_recognizer: IN_AADHAAR #1256

merged 21 commits into from
Jan 30, 2024

Conversation

devopam
Copy link
Contributor

@devopam devopam commented Jan 16, 2024

Change Description

Add a new recognizer - IN_AADHAAR, language: en

Describe your changes
Add Govt of India issued unique person identifier : Aadhaar

Issue reference

This PR fixes issue #XX

Checklist

  • I have reviewed the contribution guidelines
  • I have signed the CLA (if required)
  • My code includes unit tests
  • All unit tests and lint checks pass locally
  • My PR contains documentation updates / additions if required

devopam and others added 14 commits June 26, 2023 16:39
@devopam
IN_PAN pattern recognizer
818fe90 
Added India PAN (Permanent Account Number) recognizer
@devopam
refined IN_PAN regex
87a1aae 
refined the regex for better recognition and enhanced the test cases accordingly
@devopam
Update recognizer_registry.py
8756c93 
Fixed lint error that was missed earlier.
@devopam
Fixed Lint errors
2f85d5d 
Added test cases , verification and context data
@devopam
Merge branch 'main' of https://github.com/devopam/presidio
1b47061
@devopam
Added more test cases in test_in_pan_recognizer.py
b0d1ce8 
Added negative test cases per review comments.
@devopam
Merge branch 'main' into main
b3e94ed
@devopam
Merge branch 'main' into main
838402f
@omri374
Merge branch 'main' into main
d4ae26d
@devopam
Merge branch 'main' of https://github.com/devopam/presidio
1e81cfb
@devopam
added IN_AADHAAR recognizer
88c6c1f
@devopam
Merge branch 'microsoft:main' into main
b4edab4
@devopam
Update in_aadhaar_recognizer.py
2d01bd0 
linted code
@devopam
Merge branch 'main' into main
b7c6e65
@devopam
Copy link
Contributor Author

devopam commented Jan 17, 2024

I would like to know where to set the default score for a newly introduced recognizer.

omri374
omri374 reviewed Jan 17, 2024
View reviewed changes
Copy link
Contributor

@omri374 omri374 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! Left a couple of comments

@devopam
Update in_aadhaar_recognizer.py
2434bb5 
update pattern recognizer value per suggestion in review
@devopam
Copy link
Contributor Author

devopam commented Jan 18, 2024

hi @omri374 ,
I added a new python library : stdnum for use. Will it automatically get added to requirements.txt or I need to manually refer it somewhere ? Please advise

@omri374 omri374 closed this Jan 18, 2024
@omri374 omri374 reopened this Jan 18, 2024
@omri374
Copy link
Contributor

omri374 commented Jan 18, 2024

Hi @devopam unfortunately we cannot use stdnum because of its license. Are you familiar with another implementation with a less strict license?

@devopam
Copy link
Contributor Author

devopam commented Jan 19, 2024

Hi @omri374 ,
Nice catch on stdnum , I checked their licensing and it's actually a bit conflicting (scripts vs LGPL on pip) but nonetheless it is out of my purview. verhoeff is a standard algo, i will add it as a function in main class itself and re-submit.

@omri374
Copy link
Contributor

omri374 commented Jan 19, 2024

I think that's a good way forward. Perhaps create a checksum or validation file with the verhoeff algorithm? We may add more to it in the future.

@devopam
added utility function class
b6db593 
added PresidioAnalyzerUtils class with generic functions. removed usage of stdnum
@devopam
Copy link
Contributor Author

devopam commented Jan 23, 2024

Hi @omri374 ,

I have added analyzer_utils.py as a generic class for utility functions, and created the logic for verhoeff validation in it as a function. Please have a look and let me know .

omri374
omri374 reviewed Jan 28, 2024
View reviewed changes
Copy link
Contributor

@omri374 omri374 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks great. Left one comment asking to add a few unit tests to the utils class.

omri374 and others added 2 commits January 28, 2024 11:34
@omri374
Merge branch 'main' into main
dfb2d26
@devopam
Create test_analyzer_utils.py
fd28708 
added test cases for analyzer_utils.py in prescribed format
@omri374
Copy link
Contributor

omri374 commented Jan 29, 2024

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

omri374
omri374 previously approved these changes Jan 29, 2024
View reviewed changes
@devopam
Update test_recognizer_registry.py
f0c9737 
added to the count of predefined recognizers
@devopam
Copy link
Contributor Author

devopam commented Jan 29, 2024

hi @omri374 . I updated the failing test case with increased count of predefined recognizers. kindly run once again. regret the rework.

@omri374
Copy link
Contributor

omri374 commented Jan 29, 2024

/azp run

@omri374
Copy link
Contributor

omri374 commented Jan 29, 2024

no worries :)

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@omri374
Copy link
Contributor

omri374 commented Jan 30, 2024

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

omri374
omri374 approved these changes Jan 30, 2024
View reviewed changes
Copy link
Contributor

@omri374 omri374 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@omri374 omri374 merged commit 4008f36 into microsoft:main Jan 30, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@omri374 omri374 omri374 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@devopam @omri374