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

Tabulator: keywords and like filters aren't regex #4423

Merged
merged 1 commit into from
Feb 8, 2023

Conversation

maximlt
Copy link
Member

@maximlt maximlt commented Feb 8, 2023

Tabulator JS defines the like and keywords filters as follows:

The like filter displays any rows with data that contains the specified string anywhere in the specified field. (case insensitive)

The keywords filter displays any rows with data containing any space separated words in the specified string (case insensitive)

These filters were implemented with Pandas' .str.contains method that by default expects the pattern to be a regular expression. This PR sets the Pandas regex parameter to False to avoid that. Additionally it sets the case parameter to False too as as described in Tabulator JS' docs these two filters are case insensitive. There was no issue with that as all the strings compared were lower-cased (I believe .casefold() is a better fit to compare strings ignoring their case), it's just a little cleaner.

Copy link
Member

@philippjfr philippjfr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and makes sense. A test would be great!

@codecov
Copy link

codecov bot commented Feb 8, 2023

Codecov Report

Merging #4423 (9c7a5d7) into main (077b86e) will decrease coverage by 0.20%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##             main    #4423      +/-   ##
==========================================
- Coverage   81.80%   81.60%   -0.20%     
==========================================
  Files         238      238              
  Lines       34872    34872              
==========================================
- Hits        28526    28458      -68     
- Misses       6346     6414      +68     
Flag Coverage Δ
ui-tests 35.96% <0.00%> (+<0.01%) ⬆️
unitexamples-tests 73.37% <80.00%> (-0.21%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
panel/widgets/tables.py 88.80% <80.00%> (ø)
panel/tests/widgets/test_terminal.py 36.44% <0.00%> (-22.43%) ⬇️
panel/widgets/terminal.py 50.47% <0.00%> (-20.48%) ⬇️
panel/io/state.py 63.02% <0.00%> (-0.53%) ⬇️
panel/tests/util.py 91.89% <0.00%> (+1.80%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@philippjfr philippjfr merged commit 499ef0f into main Feb 8, 2023
@philippjfr philippjfr deleted the fix_tabulator_client_filters branch February 8, 2023 19:57
@philippjfr
Copy link
Member

Oops, meant to merge another PR but was on my phone. Just add the test sometime.

maximlt added a commit that referenced this pull request Feb 22, 2023
maximlt added a commit that referenced this pull request Feb 22, 2023
@maximlt maximlt mentioned this pull request Feb 22, 2023
maximlt added a commit that referenced this pull request Mar 3, 2023
* keywords and like filters aren't regex (#4423)
* add examples folder dynamically added to the distrib (#4484)
* Small bug fixes  (#4481)
* use the right name for the method that gets the image data
* Bump cryptography in /examples/apps/django_multi_apps (#4418)
* Use latest react-grid from CDN (#4461)
* Fix url
* Fix React template
* prepare release notes
* Modify sys.path when running inside Jupyter Kernel (#4489)
* Add support for altair and vega-lite v5 (#4488)
* Do not re-create Vega.selections object unless selections changed (#4497)
* Bump panel.js version to 0.14.4-rc.2

---------

Co-authored-by: Simon Høxbro Hansen <simon.hansen@me.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Philipp Rudiger <prudiger@anaconda.com>
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