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

Feature/add single comparison column validation check #2160

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

ThomasHepworth
Copy link
Contributor

Type of PR

  • BUG
  • FEAT
  • MAINT
  • DOC

Is your Pull Request linked to an existing Issue or Pull Request?

Adds a validation check to assess whether any input data frames entered by a user have only a single comparison column - #1392.

Give a brief description for the solution you have provided

This function adds a basic check to determine whether any of the user's data frames contain only one comparison column after excluding the unique_id and source_dataset columns.

Where this is true, Splink will generate invalid SQL, leading to cryptic SQL errors that may be difficult to interpret.

NB: this check only activates if the user has correctly specified unique_id and source_dataset. Should there be any mistakes in entering these variables in the settings, other validation checks will identify and highlight these errors.

PR Checklist

  • Added documentation for changes
  • Added feature to example notebooks or tutorial (if appropriate)
  • Added tests (if appropriate)
  • Updated CHANGELOG.md (if appropriate)
  • Made changes based off the latest version of Splink
  • Run the linter
  • Run the spellchecker (if appropriate)

@ThomasHepworth
Copy link
Contributor Author

ThomasHepworth commented May 1, 2024

Ah oops, I forgot to check if any of our tests are affected by this logic...

@ThomasHepworth
Copy link
Contributor Author

@RobinL could you let me know if you even want this change in Splink3. I can port the code to v4 and ignore this version if preferred.

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

Successfully merging this pull request may close these issues.

3 participants