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

Add invalid_uid_lei validation #11

Closed
hkeeler opened this issue Jul 20, 2023 · 6 comments · Fixed by #30
Closed

Add invalid_uid_lei validation #11

hkeeler opened this issue Jul 20, 2023 · 6 comments · Fixed by #30
Assignees

Comments

@hkeeler
Copy link
Member

hkeeler commented Jul 20, 2023

Add validation to handle invalid_uid_lei

Acceptance Criteria:

  • invalid_uid_lei validation has been added
  • Passing validations on good/bad test data
@aharjati
Copy link
Contributor

aharjati commented Aug 9, 2023

from our dev-touchpoint-meeting, this validation should take in list of valid LEIs and then use the list to verify existing LEI validity

@hkeeler
Copy link
Member Author

hkeeler commented Aug 9, 2023

In most cases it shouldn't be a list, just a single value that's the LEI for the institution doing the filing. However, there may be scenarios where an institution changes its LEI mid-year (mergers, etc.), resulting in multiple valid LEIs. We should probably chat with David if that's a possibility. If yes, that likely complicates things, and we'd need a way of resolving the current institution's LEI with other LEIs that'd also be valid for them for a filing year.

@hkeeler
Copy link
Member Author

hkeeler commented Aug 10, 2023

Got confirmation that this validation really is as simple as it reads.

The first 20 characters of the 'unique identifier' should match the Legal Entity Identifier (LEI) for the financial institution.

So, the question is how do we pass the institution's LEI into Pandera's schema? In previous discussion, we said we'd just make that Check settings be dynamic. However, sblar_schema is statically set and referenced directly. To make this dynamic, we're probably going to need a wrapper class so we can pass the LEI value into it.

Alternatively, we'd talked about creating a "validation context", perhaps using a thread local or contextvar. We'd rather not go that way as it's kinda "magic", and trickier to write tests for.

@hkeeler
Copy link
Member Author

hkeeler commented Aug 16, 2023

I know we'd talked about adding the skip validation logic here on this issue, but it's not really directly related to the validation. It really comes into play when we users want to validate anonymously, like on validation #29. We could work on that there, or a separate issue.

@nargis-sultani
Copy link
Contributor

I think being able to skip this validation logic is already added here.

@nargis-sultani
Copy link
Contributor

@aharjati Can you please comment on what you really wanted to confirm regarding this?

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 a pull request may close this issue.

3 participants