Add support for per-column custom comparators

[bryevdv]

• issues:
  • fixes [FEATURE] Custom sorting of columns in a table #13893
  • fixes [FEATURE] Option for DataTable column sorting to place NaNs last #11104
• tests added / passed
• release document entry (if new feature or API change)

@bokeh/dev This would still needs tests, docs, and examples, but I wanted to get some early feedback on the approach, which seems to work. I am also sure the TypeScript will needs improvements, and I am also not sure how if at all the current "default" sorter code needs to be considered (i.e. whatever it was trying to do with nans already seem to not be desired, can it just be jettisoned?)

Here is a test script based on the OP:

from bokeh.io import show
from bokeh.models import ColumnDataSource, DataTable, TableColumn, CustomJSCompare

source = ColumnDataSource(data=dict(foo=["AB 1", "AB 10", "AB 2"]))

sorter = CustomJSCompare(code="""
    const xn = Number(x.split(" ")[1])
    const yn = Number(y.split(" ")[1])
    if (xn == yn) { return 0 }
    else if (xn < yn) { return -1 }
    else return 1
""")
columns = [
    TableColumn(field="foo", title="Foo", sorter=sorter),
]
data_table = DataTable(source=source, columns=columns, width=400, height=280)

show(data_table)

With results for:

• Ascending

  

• Descending
  

[mattpap]

isNaN() can somewhat work in place of isNumber(), but it's finicky, and only as long as any type is involved on the type-level. Given the inputs to this method can be anything, I would change the types to unknown and add isNumber() guards to make sure isNaN() has actual numbers to work with.

[bryevdv]

I made this change but it also limited the comparison in general to numeric types, I could have imagined sorting nans first or last relative to non-numeric types in case someone used nan as "missing". Currently returning 0 for non-numeric types, but perhaps an error is also an option?

[bryevdv]

@mattpap we are not currently using DataView correct? Does that seem like something worth pursuing in the future, or did you intend to look at a new/different table implementation entirely at some point?

[mosc9575]

Some comments on the docstrings to make sure they won't be missed.

[bryevdv]

@bokeh/dev thoughts on what level of docs support this needs? Will definitely add basic reference guide materials. Some other possibilities:

• Add a comparision to some table example or examples (which ones?)
• Add a dedicated user guide sub-section on comparisons (where?)
• Combination of the above...

@mattpap any guidance on BokehJS tests? Can you point me towards something analogous that I can use as a basis to emulate?

[bryevdv]

I've added basic ref docs. Absent feedback about other docs or tests I am marking this ready.

[mattpap]

we are not currently using DataView correct? Does that seem like something worth pursuing in the future, or did you intend to look at a new/different table implementation entirely at some point?

Apparently not. I don't have any plans for data tables besides updating SlickGrid to a newer version (PR #13522).

[mattpap]

Here is a test script based on the OP: (...)

I would turn this sample code into an example under examples/interaction/widgets.

[mattpap]

any guidance on BokehJS tests? Can you point me towards something analogous that I can use as a basis to emulate?

There's a single good test that can be a basis for this:

bokeh/bokehjs/test/integration/tables.ts

Lines 9 to 21 in 8c2833e

	it("should allow sorting a non-selectable table", async () => {
	const source = new ColumnDataSource({data: {foo: [10, 20, 30, 40], bar: [3.4, 1.2, 0, -10]}})
	const foo_col = new TableColumn({field: "foo", title: "Foo", width: 200})
	const bar_col = new TableColumn({field: "bar", title: "Bar", width: 200})
	const columns = [foo_col, bar_col]
	const table = new DataTable({source, columns, sortable: true, selectable: false})
	const {view} = await display(table, [600, 400])
	const el = view.shadow_el.querySelectorAll(".slick-header-column")[2]
	await mouse_click(el)
	await view.ready

Put the construction code in a function, configure with NanSorter and CustomJSCompare, then have a test with an image captured with default sorting and another after a click on a header. That should be cheap and easy to cover the basics.

[bryevdv]

Apparently not. I don't have any plans for data tables besides updating SlickGrid to a newer version (PR #13522).

I'll make an issue soon. It's probably something worth looking into at some point so that we can jettison our adhoc homegrown view

[mattpap]

Looks good.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.