Refactor: Queries module code refactor #117
Merged
Conversation
This file contains 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
This was referenced Mar 17, 2022
syphax-bouazzouni
changed the title
syphax-bouazzouni
changed the title
|
Some tests ARE failing at the REGEX functionality. Looks like #116 introduced these lines: But I can't find that code in #117. The failure we are getting is: |
syphax-bouazzouni
force-pushed
the
pl/sparql-query-code-refactor
branch
from
fa76894 to
9f88e07
Compare
Codecov Report
@@ Coverage Diff @@
## master #117 +/- ##
==========================================
- Coverage 85.27% 82.61% -2.67%
==========================================
Files 20 24 +4
Lines 2038 2266 +228
==========================================
+ Hits 1738 1872 +134
- Misses 300 394 +94
|
|
Hi, it is fixed with (9f88e07) I think the issue was that when mergin the REGEX code got remvoed. So now all the tests pass for 4store. |
|
We have not made AllegroGraph tests to pass yet so please ignore that for the time being |
|
@syphax-bouazzouni thanks for the quick fix |
|
(9f88e07) fixes fourstore tests but introduces new type of an error in AllegroGraph before: 102 tests, 1396 assertions, 8 failures, 0 errors, 16 skips Example of the new type of error: |
|
Hi @alexskr , @mdorf This version of Algregorapah does not know the used option Removing those lines passes the tests but breaks others that are harder to debug. |
|
I re-run tests in my environment with |
|
Yes, this patch will be part of AllegroGraph v7.4
On Jul 30, 2023, at 4:34 PM, Alex Skrenchuk ***@***.******@***.***>> wrote:
I re-run tests in my environment with rfe17161-7.3.1.fasl.patch in place and this particular issue got resolved.
currently published develop version of agraph (v7.4.0-devel-2023-06-15) doesn't include this patch. I expect AGraph v7.4.0 to contain this patch when it is released.
—
Reply to this email directly, view it on GitHub<#117 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAP2LDXX4NHHWIJBLKD6YNTXS3VQZANCNFSM5QNJTFOQ>.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>
…_______________________________________________
bioontology-admin mailing list
***@***.******@***.***>
https://mailman.stanford.edu/mailman/listinfo/bioontology-admin
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What
I refactored the code of the function model_load (used to build the SPARQL query and map the results into models), from a function with 600 lines that don't follow the Single responsibility principle, to two main classes (with functions that have less than 100 lines). The first one (QueryBuilder) builds the select query and the second one (SolutionMapper) maps the query results into models.
Why
I think that this refactor will make the code more understandable for normal humans (and new newcomers) and more open for extension.
Tests
Pass all the tests of test_where.rb