Skip to content

Ensure consistent and robust sorting #129

Description

@apdavison

Result ordering is currently implicit, undocumented on our side, partly accidental, and in one respect invalid. Four related problems.

1. Generated queries request two sort keys, which the API forbids

The KG query API documentation is explicit:

The specification of the "sort": true flag on one of the properties therefore allows to sort the instances by the value of this property in an ascending order. Please note, that you can specify "sort": true on only one property and only on the root level.

KGObject.generate_query() adds sorting in a third pass (fairgraph/kgobject.py:1036):

for prop in query.properties:
    if prop.name in ("name", "fullName", "lookupLabel"):
        prop.sorted = True

It flags every matching top-level property rather than choosing one, so a class with more than one produces two "sort": true entries and breaches the documented constraint. Affected are the classes having both name and lookup_label:

  • v4: ParcellationEntity, ParcellationEntityVersion, SlicingDevice, Electrode, ElectrodeArray, Pipette
  • v5: ParcellationEntity, ParcellationEntityVersion

The reason this has gone unnoticed is that the KG is lenient: tested against pre-production, the query is accepted without error and the first sorted property in document order is honoured while the second is ignored. Running the same ParcellationEntity query with the two swapped changes the ordering from lookupLabel to name, confirming that position decides it. That leniency is not something to rely on — the spec says one property, and only the root level.

Which property wins is itself accidental: the builder emits properties alphabetically, so lookup_label beats name. Nothing chose that, and a change in property ordering would silently change how these classes are ordered.

(The root-level restriction is satisfied today, since the pass iterates only over top-level properties — but by construction rather than by intent, and nothing tests it. The documentation notes that flattening is the supported way to sort by a nested value, should that ever be wanted.)

2. The sortable set is narrower than the name-like set

The sort tuple lists three property names, while by_name() treats seven as name-like: name, lookup_label, family_name, full_name, short_name, abbreviation, synonyms. Classes named by any other means get no sorting at all — Person.generate_query() emits zero "sort": true, so listing people returns them in whatever order the KG supplies.

Whether that is intended is one of the things this issue should settle.

3. Nothing about ordering is documented in fairgraph

There is no mention of sorting anywhere in the user documentation, and list()'s docstring says nothing about the order of its results. A user has no way to know whether DatasetVersion.list(client) is ordered, by what, or whether the order is stable between calls. The only reference is the sorted parameter of QueryProperty in the API reference, which is internal.

The KG documents its own contract, so we have something firm to document against: exactly one root-level property, ascending. Worth distinguishing the separate ensure_order parameter, which concerns preserving the order of a list-valued property rather than sorting results.

4. Nothing is tested

No test asserts how many sort keys a generated query carries, which property is chosen, or that results come back in the expected order. Since the behaviour depends on a server contract we do not control, it should be pinned from both sides:

  • offline, over the golden query fixtures: at most one "sort": true, at the root level, on the expected property
  • live, against pre-production: results actually arrive ordered by that property

The live test is what tells us if the KG ever changes how it treats sorting — including if it stops being lenient about the current breach.

Note when writing assertions that the KG sorts case-insensitively: AAL1_brain falls between AAL1_AMYG and AAL1_CAU, so a plain Python sorted() will disagree.

Done when

  • exactly one root-level sort property is chosen, deliberately and in a documented priority order
  • the relationship between sortable and name-like properties is settled, whether by widening the sortable set or documenting why it is narrower
  • ordering is described in the user documentation and in list()'s docstring
  • offline and live tests pin both fairgraph's choice and the KG's behaviour
  • the note at fairgraph/queries.py:314 is removed

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions