-
Notifications
You must be signed in to change notification settings - Fork 71
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
complete workflow to check sparql queries #396
complete workflow to check sparql queries #396
Conversation
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 |
@andrewtavis, I've finally made the workflow file to work! (omg!!) but how should the expected behaviour be like? For now, it only logs to the console that the files that have QID issues but the workflow does not actually fail so all green checks. How do we wanna approach this? Should the workflow fail? is there a way we could notify the PR author that something is wrong? |
The workflow should fail :) A big thing is that it's also a tool for maintainers to know if things are wrong in a PR, and and a failed workflow says that loud and clear. We were discussing warnings for GitHub actions in another issue, which apparently are a thing, but let's keep it all to if there's a problem fail the workflow. |
Okay, I'll figure this out. Thanks for the update @andrewtavis! |
@andrewtavis I have updated the codebase to cause the workflow to fail once we have invalid queries. with this: # Exit with an error code if any incorrect QIDs are found
if incorrect_languages or incorrect_data_types:
sys.exit(1) I believe with this, all checks are complete |
Quick note being sent to all the testing PRs, if updates are needed now that #402 has been merged, then it'd be great to get those updates to the branch :) If no updates are needed, then let me know 😊 |
checking... |
I noticed that there was no folder for Igbo.
added comparative
- Utilized already built helper functions to support sub-languages when retrieving ISO and QID values. - Updated table printing to correctly format and display both main languages and sub-languages.
…tion to reflect the new JSON structure, ensuring only data types are printed and no sub-languages unlike before.
…_name' to align with the directory structure in the language_data_extraction directory.
…tion to handle sub_language folders.
…se list_all_languages, assigning a complete list of all languages.
…uages listing functions
- Updated all test cases to account for sub-languages. - Removed tests for est_get_language_words_to_remove and est_get_language_words_to_ignore, as these functions were deleted from utils.py and the languages metadata files
…. Made the language_metadata parameter optional in two functions. Added a ValueError exception when a language is not found.
- Positive and negative tests for format_sublanguage_name - Test to validate the output of list_all_languages
…ON structure - Updated the logic for building language_map and language_to_qid to handle languages with sub-languages. - Both main languages and sub-languages are now processed in a single pass, ensuring that: - language_map includes all metadata for main and sub-languages. - language_to_qid correctly maps both main and sub-languages to their QIDs.
…uages listing functions
…ON structure - Updated the logic for building language_map and language_to_qid to handle languages with sub-languages. - Both main languages and sub-languages are now processed in a single pass, ensuring that: - language_map includes all metadata for main and sub-languages. - language_to_qid correctly maps both main and sub-languages to their QIDs.
…ON structure - Updated the logic for building language_map and language_to_qid to handle languages with sub-languages. - Both main languages and sub-languages are now processed in a single pass, ensuring that: - language_map includes all metadata for main and sub-languages. - language_to_qid correctly maps both main and sub-languages to their QIDs.
Hi @andrewtavis I have adjusted this PR to accommodate the changes from #402 and as you can see a workflow is failing. And that is the one to Check Query Identifiers. For now, these are the files causing the issue (you can check the workflow message):
They are failing because they do not exist in the |
hi @andrewtavis 👋🏾 I also updated the test cases to ensure we expect these languages Now all test cases are passing :) We can say that all QIDs are valid. Specifically, language QIDs and data type QIDs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really great work here, @DeleMike 😊 Appreciate your dedication to getting these tests up and running! 🚀
Contributor checklist
Description
Adds steps in workflow file to run
check_query_identifiers.py
Related issue