-
Notifications
You must be signed in to change notification settings - Fork 5
DRAFT: Filtering on columns #44
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
base: main
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
|
I realize this isn't fit to be merged in, it's a copy of the file I'm using in my codebase for building documentation (the setup function shouldn't be there in this repo). Also, it's been a while since I made this and I find it hard to make sense of my own code at this point (I think it's been mainly a bunch of copy-pasting from other parts until it worked). Regardless, it works for my current use case and maybe it can serve to demonstrate the intent if nothing else. Feel free to close the PR if you have no more need for it and wish to start from scratch. I definitely don't need credit for it. |
|
I dug a bit deeper in my original implementation and cleaned up the code a bit, also added comments. Now that my memory is a bit refreshed, an issue I found with my implementation is the naming of the options. I think in order to integrate column filtering properly, the existing options should be renamed ("include" --> "include_row_regex", "included_cols" --> "include_col_index",...) to make them less confusing. |
|
Dear Bavo, thanks a stack for sharing your code. Maybe @aalapd will find it useful already (#38). We will take it into consideration on the next development iteration, together with your suggestions. With kind regards, P.S.: I've just added a few adjustments to satisfy the linter on your patch. |
|
Hi. Because the patch might still be a draft, I am toggling it into draft mode. However, if you see fit, and if it breaks nothing else, I will be happy to merge & release.
That would probably be the main challenge: Re-learn how the code does its job, and accompany it by a few corresponding software tests. If anyone in the community would like to see this land, contributions are much appreciated. 🍀 |
Coming from #35, this patch is what I am using to allow include/exclude filtering of columns with regex patterns.