Skip to content

Add table_id argument#36

Merged
eutwt merged 23 commits intomainfrom
table-id
Nov 15, 2025
Merged

Add table_id argument#36
eutwt merged 23 commits intomainfrom
table-id

Conversation

@eutwt
Copy link
Owner

@eutwt eutwt commented Oct 12, 2025

This PR adds the table_id argument to compare(), with a slightly different approach than #34. The functions which take a comparison argument all use the comparison's table_id, rather than having their own argument to specify table identifiers.

@elipousson Thanks for the feature request and initial implementation! Could you let me know what you think of this approach?

Edit: On second thought, the suffix argument was a good idea. So this PR is now the same approach on the user end as yours. The only real difference with yours is I kept things as "a" and "b" for steps if they were entirely internal (locate_matches), and handled some rare edge cases like a suffix vector where both values are the same.

@codecov
Copy link

codecov bot commented Oct 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (1da3111) to head (8533058).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #36   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines          394       439   +45     
=========================================
+ Hits           394       439   +45     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@elipousson
Copy link

This looks great! I'll test it as soon as I get a chance and be sure to report back if I can any unexpected issues. Thanks for being open to the contribution.

@eutwt eutwt merged commit 185fed3 into main Nov 15, 2025
7 checks passed
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.

2 participants