Add abstract CSV filters, filter overrides for specific lookup types

[rpkilby]

This supersedes/builds on my last PR (#362). Here is the diff between the two.

Relevant issues:

Closes #362 (PR)
Closes #344 (PR)
Fixes #321
Fixes #273

Changes:

• adds the ability to use transforms in lookup expressions (from previous PR)
• adds abstract base classes for CSV (in, range) filters and fields.
• adds filter generation handling for lookups (Filterset.filter_for_lookup).

The abstract CSV filter work is partially based on the work done in #344. The major difference is that type conversion/validation is performed by the field instead of in the widget's customize method. Dynamically created subclasses allow us to use the abstract CSV filter to handle just the list of values, while reusing the cleaning behavior on individual values from the base field class. I also removed sanitize, as it did not allow you to filter by empty char values.

TODO:

This is not fully complete, but wanted to go ahead and try to get eyes on as it should be close to done.

• test additional field types (datetimes, strings, FKs) for abstract CSV filters
• testing for lookup overrides (Filterset.filter_for_lookup)
• documentation
• decide if dynamic CSV filters/fields should be cached (as two NumberRangeFilter classes would be identical). That said, it seems like a one time cost to generate the class then instance for each filter. Thoughts on this?

[carltongibson]

@rpkilby This looks great. Just on your last point: I'm not too worried about caching. I'd be very surprised if we had a bottleneck here. (And I'm inclined to leave such things as out of scope.)

[rpkilby]

Hi @carltongibson. I think this is ready for review. My only outstanding concern is documentation. I have added docs for the base IN and RANGE filters, as well as doc updates for the shift from lookup_type to a lookup_expr that supports transforms.

So, are more extensive docs necessary? While this is a somewhat large PR, it's really just one big bug fix for handling filter generation for some lookups. Maybe adding more detailed notes to the changelog would suffice?

[carltongibson]

@rpkilby This is great.

So, are more extensive docs necessary?

What I'd really like to see here is just few lines demonstrating the usage of the (various) CSV options — by that I mean a snippet of the URL query string that the user constructs for the filtering behaviour and the matching filter declaration — i.e. we don't need running examples — but useful extracts.

My main issue is that new users find it hard to see how to put filter sets together — that's a documentation thing — the learning curve is too steep.

Makes sense?

[rpkilby]

@carltongibson I've added docs on a separate branch/self-PR because the changes are somewhat extensive. Similar to this PR, there's a bit going on so I tried to isolate the work done in each commit. Reading the history in order might give you a better idea of what's going on than looking at the whole diff at once.

• documentation PR on my fork here.
• build of the docs hosted on gh-pages here.

What I'd really like to see here is just few lines demonstrating the usage of the (various) CSV options

Usage docs for the CSV filters are in this commit. This was based off of the DateTimeFromToRangeFilter docs.

My main issue is that new users find it hard to see how to put filter sets together

All of the other commits are an attempt at improving the structure and approachability of the docs. I don't know if that's achieved, but that was the idea. Please nitpick any issues or submit PRs against my fork or whatever.

If all looks good, I'll merge the doc changes PR into this one so we can proceed.

[rpkilby]

Also, added some notes on how to override filter_for_lookup. This is also used to explain the method default behavior.

[rpkilby]

Alright - docs are updated and merged.

[rpkilby]

Quick note - improved handling for isnull lookups. Instead of automatically returning a BooleanFilter with the default arguments, this redoes the DEFAULTS lookup as if it were a boolean field. This ensures 'isnull' lookups conform to any changes in filter_overrides. eg, using the ajax friendly BooleanWidget.

The in and range lookups are still correct, as their underlying model field type is the same.

[carltongibson]

👍

[rpkilby]

Sweet - any timeframe on the release? There are some updates to drf-filters that will be dependent on it. Thanks.

[carltongibson]

This week.

[rpkilby]

Great - thanks again.

[mlissner]

This is an old thread, but I just want to say how wonderful it was to find this feature available today, just when I needed it. Truly wonderful, thank you so much for all your efforts on this project. You've made at least one person super happy today.

[rpkilby]

😊