Allow to set facets_array in metadata (like current facets)

[davidbgk]

For now, you can set a facets value (array) in your metadata file but I couldn't find a way to set a facets_array in order to provide default facets for arrays (like tags). My use-case is to access to that kind of view by default without URL's parameters as with other default facets.

I'm new to datasette, and I'm willing to help with a PR if that is not already implemented and I missed it!

[simonw]

This is definitely a missing feature. The "different types of facet" stuff feels incomplete to me generally - this is one issue, but this one as well:

• If you apply ?_facet_array=tags then &_facet=tags does nothing #625

[davidbgk]

@simonw thank you for your fast answer and your guidance!

While digging into the code, I found an undocumented way of doing it:

facets: ["Facet for a column", {"array": "Facet for an array"}]

The only remaining problem with that solution is here:

datasette/datasette/facets.py

Line 33 in 250db81

type, metadata_config = metadata_config.items()[0]

We have:

type, metadata_config = metadata_config.items()[0]

But it requires to cast the dict_items as a list prior to access the first element:

type, metadata_config = list(metadata_config.items())[0]

I guess it's an unspotted bug? (I mean, independently of the facets-with-arrays issue.)

[simonw]

I think you're right! I had completely forgotten that piece of code.

This just turned into a bug fix and a documentation update. Thanks for the research!

[simonw]

I tried that fix you suggested and now this metadata.json does the right thing:

{
    "databases": {
        "fixtures": {
            "tables": {
                "facetable": {
                    "facets": [
                        {
                            "array": "tags"
                        }
                    ]
                }
            }
        }
    }
}

It does further highlight the bug in #625 though - since then if you try to add ?_facet=tags to facet by tags treating them NOT as an array your request to do so is ignored.

[simonw]

I'm also not convinced that this configuration syntax is right. It's a bit weird having a "facets" list that can either by column-name-strings or {"type-of-facet": "column-name"} objects. Maybe there's a better design for this?

Part of the problem here is that facets were designed to accept optional extra configuration - partly to support m2m facets in #495 - but I haven't actually shipped any facets that use that ability.

Facet by delimiter would be a good one to exercise that ability:

• Ability to facet by delimiter (e.g. comma separated fields) #510

[simonw]

I'm going to ship your fix now, but I'm not going to add this to the documentation yet because I hope to improve the design prior to Datasette 1.0.

[simonw]

... actually no, I WILL document this, because not documenting this is what got us to this point in the first place!

[simonw]

Updated documentation: https://github.com/simonw/datasette/blob/20a2ed6bec367d2f6759be4a879364a72780b59d/docs/facets.rst#facets-in-metadatajson

[davidbgk]

Wow, that was fast, thank you so much @simonw !

I'm also not convinced that this configuration syntax is right. It's a bit weird having a "facets" list that can either by column-name-strings or {"type-of-facet": "column-name"} objects. Maybe there's a better design for this?

I agree that it's not ideal, my initial naive approach was to detect if it's an array, like what is done here:

datasette/datasette/facets.py

Lines 312 to 313 in 2c07327

	types = tuple(r[0] for r in results.rows)
	if types in (("array",), ("array", None)):

But it requires an extra query to determine the type, which is a bit problematic, especially for big tables I guess.

Taking a look at #510, I wonder if a facet_delimiter should be defined for that kind of columns (that would help our team not to have an intermediary conversion step from foo|bar to ["foo","bar"] for instance).

To be consistent with the --extract-column parameter, maybe an explicit casting/delimiter would be useful: --set-column 'Foo:Array:|'.

Throwing a lot of ideas without knowing the big picture… but sometimes newcomers have superpowers :).