Skip to content

Tweaks to table row matching with numerics #113

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

Merged
merged 2 commits into from
Jun 27, 2025
Merged

Tweaks to table row matching with numerics #113

merged 2 commits into from
Jun 27, 2025

Conversation

ctmay4
Copy link
Member

@ctmay4 ctmay4 commented Jan 21, 2025

When matching table rows, we are using a float comparison for ranges. That means that we can find a match for a decimal value for a purely integer range. That is not what was intended.

For example, if a table had a row defined like this:

"rows" : [ [ "000-120,999" ] ]

If you tried to match "35.5" it would find a match because it was just doing a float compare between "0" and "120". However since that is not a decimal comparison it should not have found a match. This issue changes so that if neither the high or low value have a decimal point and the value it is using to compare does, then it will not find a match. So "35.5" would not find a match.

@ctmay4 ctmay4 added the bug label Jan 21, 2025
@ctmay4 ctmay4 self-assigned this Jan 21, 2025
Copy link

@ctmay4
Copy link
Member Author

ctmay4 commented May 23, 2025

@schusslern I remember we talked about this being an issue and I created this merge request. I cannot remember why I did not merge this. I assume this is something you want?

@schusslern
Copy link

schusslern commented May 23, 2025 via email

@ctmay4
Copy link
Member Author

ctmay4 commented May 23, 2025

Sounds good. I just wanted to make sure this was not going to break something you were relying on.

@ctmay4 ctmay4 merged commit 6a5439a into master Jun 27, 2025
4 checks passed
@ctmay4 ctmay4 deleted the numeric-contains branch June 27, 2025 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants