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

Add functionality for sorting facets/observations into a tree #1542

Open
wants to merge 17 commits into
base: develop
Choose a base branch
from

Conversation

lrosenstrom
Copy link
Contributor

Maybe we can try this out on dev2 to see how it works in practice.

@lrosenstrom lrosenstrom requested review from olovy and kwahlin December 18, 2024 15:17
@lrosenstrom
Copy link
Contributor Author

This might be a too naive approach to how this should work? Hopefully it can be a start...

Copy link
Contributor

@kwahlin kwahlin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments, otherwise LGTM! Let's try it out on e.g. dev2.


SearchUtils2(Whelk whelk) {
this.queryUtil = new QueryUtil(whelk);
this.disambiguate = new Disambiguate(whelk);
this.jsonLd = whelk.getJsonld();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to pass on the JsonLd object, use Disambiguate.isSubclassOf() instead.

@@ -123,6 +127,9 @@ private Map<String, Object> buildSliceByDimension(Map<Property, Map<PropertyValu
var sliceNode = new LinkedHashMap<>();
var isRange = rangeProps.contains(property);
var observations = getObservations(buckets, isRange ? queryTree.removeTopLevelPropValueWithRangeIfPropEquals(property) : queryTree, nonQueryParams);
if (property.name().equals(Disambiguate.RDF_TYPE)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use property.isType() to catch both work and instance types (hasInstanceType/instanceOfType in addition to rdf:type).

@lrosenstrom lrosenstrom force-pushed the feature/lws-268-facettree branch from 78992be to 7d4befb Compare January 14, 2025 09:31
var children = findChildren(observation, observations);
if (!children.isEmpty()) {
queue.addAll(children);
observation.put("children", children);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
observation.put("children", children);
observation.put("_children", children);

Since children is not a defined property

@lrosenstrom lrosenstrom force-pushed the feature/lws-268-facettree branch from ff9f414 to 4994254 Compare January 16, 2025 09:47
@lrosenstrom lrosenstrom force-pushed the feature/lws-268-facettree branch from 7a7884a to 5718480 Compare January 28, 2025 15:50
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

Successfully merging this pull request may close these issues.

2 participants