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

facets: make Facet by this column option smarter #28

Closed
cldellow opened this issue Jan 28, 2023 · 11 comments
Closed

facets: make Facet by this column option smarter #28

cldellow opened this issue Jan 28, 2023 · 11 comments

Comments

@cldellow
Copy link
Owner

Split from #21

If we remove facet suggestions, we no longer have a way to facet by date, or by array.

It would be nice if we could customize the cog to show those options -- right now it always forces column faceting.

I think there isn't an official way to hook column actions yet: simonw/datasette#983 (comment)

We might be able to party in https://github.com/simonw/datasette/blob/main/datasette/static/table.js#L1

...although discovering which column we're connected too will be a pain. I think we'd have to parse the absolute positioning then reverse engineer which column it is

This also means mobile users won't be able to control facets, I'm ok with that for now -- plus we might make the omnibar smarter, which would mitigate this a bit.

@cldellow cldellow changed the title facets: make Facet by cog smarter facets: make Facet by this column option smarter Jan 28, 2023
@cldellow
Copy link
Owner Author

cldellow commented Jan 28, 2023

Idea 1: could we sniff the values client side to figure out what options to show?

Problem: render_cell will trip us up. eg we render ["foo", "bar"] as foo, bar so we'll have lost the context.

Idea 2: compute minimal summary statistics for every column in every table. Consult this to determine what the valid options are.

Problem: requires write access on the DB at some point. Won't work (I don't think?) for views unless we have a way to plumb that info through.

Ultimately, I think I like idea 2... how hard is it to plumb the view information all the way through? I think that's equivalent to the work in simonw/datasette#1293

@cldellow
Copy link
Owner Author

For my immediate use case, tables are the most important thing. For tables, we know the exact column name and can consult it to determine options.

For views, we can just do the current logic, which will be wrong in some cases. If/when we can map view columns to table columns, we can improve it down the road.

@cldellow
Copy link
Owner Author

An idea: sniff the values that pass through render_cell, jamming them on to the request object.

Then extra_body_script can include a stanza that says what the valid options are.

@cldellow
Copy link
Owner Author

This is bad, of course - it's so far outside of the facet machinery that we won't be able to re-use suggest_facet.

Hmm... Unless, and bear with me, we do something particularly disgusting, like create a transient in-memory sqlite table, stuff the values into it, then ask each facet implementation to suggest facets.

That's disgusting, I think I love it.

@cldellow
Copy link
Owner Author

That also protects against non-defensive implementations that might do accidental table scans.

A risk: if there's not enough variety in the first N things, we won't suggest a facet.

I think maybe we should just hardcode the rules.

@cldellow
Copy link
Owner Author

render_cell doesn't have access to the request, so this scheme won't work

@cldellow
Copy link
Owner Author

render_cell is also kinda poor because it gets invoked 1x per cell, not 1x per row. (Although we could probably use the id of the row to detect when a new row was passed?)

That's a shame, because this was seeming fairly elegant

How hard would it be to monkey patch Datasette to either pass the request to render_cell, or to stash the rows in the view code?

@cldellow
Copy link
Owner Author

For table views, there's this function: https://github.com/simonw/datasette/blob/0b4a28691468b5c758df74fa1d72a823813c96bf/datasette/views/table.py#L881, which seems to get a request and a set of rows.

Does that path get hit for views and custom sql queries?

@cldellow
Copy link
Owner Author

Gets called for views, not for custom sql queries.

You can't facet custom sql queries, so this is OK.

I expect this function will disappear in Simon's rewrite simonw/datasette#1518

That's OK, we can just gracefully degrade until we get a new way to access this data.

@cldellow
Copy link
Owner Author

oh, duh, that was a change that Simon just made in order to make request visible to render_cell. Sooo, it's not there currently, which is unfortunate.

@cldellow
Copy link
Owner Author

cldellow added a commit that referenced this issue Jan 28, 2023
request now has a _dux_rows field, which we'll inspect to determine
which facets we should offer to a user.
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

No branches or pull requests

1 participant