Repository navigation
Conversation
Generated queries now carry exactly one root-level sort key, chosen deliberately, instead of sorting by every name-like property. The KG query API allows 'sort' on only a single root-level property, but the previous code set it on all matching root properties, so classes with both 'name' and 'lookup_label' (ParcellationEntity, Electrode, SlicingDevice, ...) produced an invalid query with two sort keys. The sortable set is now aligned with the name-like set used by by_name(), and the single sort property is chosen from it in a fixed priority order: name, full_name, short_name, family_name, abbreviation, lookup_label (synonyms is never used). In practice this means 'name' now wins over 'lookup_label', Person gains a family_name sort key where it had none, and Dataset/Organization sort by full_name. Documents result ordering in doc/queries.rst and the KGObject.list() docstring, and adds offline tests pinning the single root-level sort key plus a live test asserting case-insensitive ordering (the KG sorts case-insensitively, so the assertion is not a plain Python sorted()). Implementation made use of DeepSeek V4 Flash via Albert API (DINUM)
ParcellationEntity and ParcellationEntityVersion now sort by lookup_label instead of name, so that terms from the same atlas stay grouped together. The default sort priority now lives in the openMINDS builder (builder/update_openminds.py, DEFAULT_SORT_PRIORITY) and is regenerated into every generated class as a SORT_PRIORITY class attribute, so that the runtime sort logic (KGObject.generate_query / choose_sort_property) reads a per-class value. A dict of exceptions, SORT_PRIORITY_EXCEPTIONS, overrides the default for specific classes; it currently contains only the two parcellation classes, which prefer lookupLabel. - builder/update_openminds.py: DEFAULT_SORT_PRIORITY + SORT_PRIORITY_EXCEPTIONS, injected into the template context. - builder/fairgraph_module_template.py.txt: emit SORT_PRIORITY on each generated class. - fairgraph/queries.py: choose_sort_property takes an explicit priority (default = module default). - fairgraph/kgobject.py: KGObject.SORT_PRIORITY default; generate_query uses the per-class value. - Regenerated v4/v5 openminds module (513 files, one added line each). - Tests: updated sort-key expectations and added a test pinning the per-class override. - Docs: queries.rst, list() docstring and release notes mention the override. Implementation made use of DeepSeek V4 Flash via Albert API (DINUM)
Rather than choosing which property to sort on at runtime for every query, decide it once in the builder and bake the single value into each generated class as a SORT_PROPERTY attribute (a camelCase schema name, or None). - builder/update_openminds.py: add get_sort_property(), which picks the first name-like property each class actually has, in DEFAULT_SORT_PRIORITY order (or the per-class SORT_PRIORITY_EXCEPTIONS override); inject it into the template context as sort_property. - builder/fairgraph_module_template.py.txt: emit SORT_PROPERTY = "<name>" or None instead of the full SORT_PRIORITY priority tuple. - fairgraph/kgobject.py: generate_query() reads cls.SORT_PROPERTY and applies it via find_sort_property(); KGObject.SORT_PROPERTY = None. list() docstring updated. - fairgraph/queries.py: drop the runtime SORT_PRIORITY constant and choose_sort_property(); add minimal find_sort_property(). - Regenerate v4 and v5 (513 files). ParcellationEntity(-Version) sort by lookup_label; default classes sort by name/fullName/familyName as before. - Update tests and docs for the build-time decision. The changes in this commit were produced by the prompt "rather than choose which property to sort on at run time, it would be more efficient to do this at build time, in the update_openminds.py script", in the same session
Member
Author
|
this converged to essentially the same solution as #153 but is slightly less elegant |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #129
This PR is part of an experiment comparing the performance of different AI coding agents. The other PR addressing the same issue is #153.
This implementation made use of DeepSeek V4 Flash via Albert API (DINUM) running in opencode. The initial prompt, in a new checkout of my fork of fairgraph, was "Please make a plan to address issue #129 from the upstream HumanBrainProject/fairgraph repository".
Unlike Claude (see #153), DeepSeek did not make the suggestion of a per-class priority in deciding which property to sort on ("name", "lookupLabel", etc.) I did not intervene, but after the first pass was complete, I asked the coding agent to implement a per-class priority, using the same approach I had suggested to Claude. The result it the second commit in this PR.