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

[Issue 3767] Replacement of language comparisons using '=' by the 'langMatches' (without whitespace changes) #365

Merged
merged 3 commits into from
Feb 17, 2023

Conversation

ghost
Copy link

@ghost ghost commented Feb 9, 2023

see also: PR3789

@ghost ghost requested review from brianjlowe and chenejac February 9, 2023 17:16
@ghost ghost force-pushed the issue-3767 branch from 310bffb to ea2eada Compare February 9, 2023 17:18
@ghost ghost force-pushed the issue-3767 branch from ea2eada to 108fdbf Compare February 9, 2023 17:19
@ghost ghost requested a review from litvinovg February 9, 2023 17:25
chenejac
chenejac previously approved these changes Feb 10, 2023
@@ -479,7 +482,7 @@ private static OntModel initializeFakeDisplayModel() {
Property saveToVarProperty = m
.getProperty(DisplayVocabulary.SAVE_TO_VAR);

m.add(dataGetter, queryProperty, QUERY_STRING_LANG); //UQAM-Optimization Using query with linguistic context
m.add(dataGetter, queryProperty, QUERY_STRING_LANG("en-US")); //UQAM-Optimization Using query with linguistic context
Copy link
Author

Choose a reason for hiding this comment

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

I am wondering, is this correct to use a hard coded en-US. The calling method initializeFakeDisplayModel and the method QUERY_STRING_LANG are both static and cannot reference the non-static class member langCtx. Previously it appeared to template the SPARQL query using a data context map with langCtx.

I have approved this previously but on second thought, am not sure it is correct. Yet the class name does not indicate anything of importance and the comments appear to indicate specfic use casees.

@ghost ghost force-pushed the issue-3767 branch from 5755d10 to a1de03f Compare February 14, 2023 17:35
Copy link
Contributor

@chenejac chenejac left a comment

Choose a reason for hiding this comment

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

@michel-heon confirmed this is working for AWS Neptun

@chenejac chenejac merged commit 15cb4d7 into vivo-project:main Feb 17, 2023
@ghost ghost deleted the issue-3767 branch February 23, 2023 19:07
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.

3 participants