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.

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

New Predefined Recognizer: IN_PAN #1100

Merged
merged 9 commits into from
Jul 11, 2023
Merged

New Predefined Recognizer: IN_PAN #1100

merged 9 commits into from
Jul 11, 2023

Conversation

devopam
Copy link
Contributor

@devopam devopam commented Jun 27, 2023

Change Description

Added presidio predefined recognizer : IN_PAN

Describe your changes
Added pattern and context based recognizer InPanRecognizer to predefined recognizers.
This recognizer is for India Permanent Account Number (PAN) which is the primary unique taxpayer identifier for all citizens.

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 added 2 commits June 26, 2023 16:39
Added India PAN (Permanent Account Number) recognizer
refined the regex for better recognition and enhanced the test cases accordingly
@devopam
Copy link
Contributor Author

devopam commented Jun 27, 2023

@microsoft-github-policy-service agree [company="{BlackhawkNetwork}"]

@devopam
Copy link
Contributor Author

devopam commented Jun 27, 2023

@microsoft-github-policy-service agree company="Blackhawk Network"

@omri374
Copy link
Contributor

omri374 commented Jun 27, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@omri374
Copy link
Contributor

omri374 commented Jun 27, 2023

@devopam please run black and flake8 to make sure there are no linting issues. Thanks!

Fixed lint error that was missed earlier.
@devopam
Copy link
Contributor Author

devopam commented Jun 28, 2023

that was a bad miss. fixed and ran lint - no errors reported on local. Also, black recommended no changes

@omri374
Copy link
Contributor

omri374 commented Jul 2, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Added negative test cases per review comments.
@omri374
Copy link
Contributor

omri374 commented Jul 4, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

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! LGTM

Copy link
Contributor

@SharonHart SharonHart 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
see my comment about adding the check-sum functionality to make the recognizer even more robust.
Also add the new entity to the documentation

@omri374
Copy link
Contributor

omri374 commented Jul 9, 2023

@devopam would you be interested in adding the checksum logic in a new PR?

@SharonHart
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

No commit pushedDate could be found for PR 1100 in repo microsoft/presidio

@omri374
Copy link
Contributor

omri374 commented Jul 11, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@omri374 omri374 merged commit 0a4c76d into microsoft:main Jul 11, 2023
@omri374
Copy link
Contributor

omri374 commented Jul 11, 2023

Mergine this one, the checksum capability would be added in a future PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants