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

A function to validate the order of variables in SPARQL based on their order in the JSON file #503

Merged

Conversation

OmarAI2003
Copy link
Contributor

…d on our lexeme_forms_json file

Contributor checklist


Description

This PR introduces a new function, check_forms_order, which orders column names based on the lexeme_forms json fie. The checks the variables returned against the ground truth file, and sorts them accordingly.

Changes:

  • New Function: sort_columns_by_reference implemented to handle the sorting of column names, ensuring correct ordering based on predefined criteria.
  • Integration: integrated The function with the main validation function of the file

The SELECT clause will be ordered hierarchically:

  1. Data type will be the first variable (if it's there).
  2. Variable length (shortest first).
  3. Lexicographical order within each length group, based on a predefined JSON list.
  4. Variables not in the JSON file like those returned by the query service will be added at the end.

Note:
The SPARQL queries diagnosed by this function have not been modified yet so checks will fail. Once the function is agreed upon I will fix and push

Related issue

Copy link

github-actions bot commented Oct 31, 2024

Thank you for the pull request!

The Scribe team will do our best to address your contribution as soon as we can. The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)

If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Data rooms once you're in. Also consider joining our bi-weekly Saturday dev syncs. It'd be great to have you!

Maintainer checklist

  • The linting and formatting workflow within the PR checks do not indicate new errors in the files changed

  • The CHANGELOG has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)

@andrewtavis andrewtavis self-requested a review November 1, 2024 22:10
@andrewtavis andrewtavis added the hacktoberfest-accepted Accepted as a part of Hacktoberfest label Nov 1, 2024
@andrewtavis
Copy link
Member

Hey @OmarAI2003 👋 Looking at this for a moment, it looks like the capitalization might be a bit off for the expected forms? It's saying that we should have a bunch of UpperCamelCase returned forms, but the forms are written in lowerCamelCase. Would you be able to take a look at this? :)

@OmarAI2003
Copy link
Contributor Author

Welcome back @andrewtavis, So I wrote the function specifically for ordering purposes and didn’t account for the exact naming conventions in the file. The message printed by the function is intended to guide the query writer to match the correct order, without focusing on case sensitivity, as that’s handled by other functions
a
Do you like the suggesting returned forms in the message to be lowerCamelCase as they should be in the file?

@OmarAI2003
Copy link
Contributor Author

So, should this be the case in the final message?
a1

@andrewtavis
Copy link
Member

Yes this is looking better, @OmarAI2003. Thanks!

@OmarAI2003
Copy link
Contributor Author

Glad to hear this!
When you are done reviewing this, just let me know so I can start updating the SPARQL files diagnosed by this function.

@andrewtavis
Copy link
Member

One final thing, @OmarAI2003: For keys like SimplePresent we need it to not be fully lowered, but rather s[0].lower() + s[1:] such that we get simplePresent at the end. Feel free to make that change and start reordering the keys with it :)

@andrewtavis
Copy link
Member

General idea here @OmarAI2003 is that once the two PRs you have up are in, that I'll then do the new formatting process :) Let me know if there's anything I can do to help!

@OmarAI2003 OmarAI2003 force-pushed the Ordering-forms-based-on-json branch from 4c2336e to 98b79c0 Compare November 9, 2024 12:21
@OmarAI2003
Copy link
Contributor Author

Hi @andrewtavis, I’ve fixed the queries. The final thing that will need to change is the MARKS.

@andrewtavis
Copy link
Member

Ok, I can do that, @OmarAI2003 :)

Copy link
Member

@andrewtavis andrewtavis left a comment

Choose a reason for hiding this comment

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

The quality of the queries really is amazing now, @OmarAI2003 😊 Really thank you so much for all of the hard work to get us to this state! 🚀

@andrewtavis andrewtavis merged commit f674d2f into scribe-org:main Nov 9, 2024
7 checks passed
@OmarAI2003 OmarAI2003 deleted the Ordering-forms-based-on-json branch November 10, 2024 12:12
@OmarAI2003 OmarAI2003 changed the title Adds a new function to validate the order of variables in SPARQL base… A function to validate the order of variables in SPARQL based on their order in the JSON file Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Accepted as a part of Hacktoberfest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants