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

Violent deprecation warnings vs sqlalchemy 1.4.46 #100

Closed
bollwyvl opened this issue Jan 20, 2023 · 12 comments
Closed

Violent deprecation warnings vs sqlalchemy 1.4.46 #100

bollwyvl opened this issue Jan 20, 2023 · 12 comments

Comments

@bollwyvl
Copy link

Use of rdflib_sqlalchemy.store.union_select with sqlalchemy 1.4.46 shows:

The legacy calling style of select() is deprecated and will be removed in SQLAlchemy 2.0.  Please use the new calling style described at select(). (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9)

It's possible to hide these errors with SQLALCHEMY_SILENCE_UBER_WARNING=1, but it can be difficult to reason about this error, and how a downstream would be able to effectively do it.

@mwatts15
Copy link
Collaborator

mwatts15 commented Jan 27, 2023 via email

@richardscholtens
Copy link
Contributor

I am updating the library to use SQLAlchemy version 2.0.23. Doing so results in the failing of multiple tests. I believe this is also has to do with the deprecation of the select() and similar constructs. I already fixed the first test and will continue to fix the rest as well.

@richardscholtens
Copy link
Contributor

@mwatts15 , so I fixed all tests but one. I'm still unable to get the test_contexts_result under test_sqlalchemy to work. The test is as follows:

class SQLATestCase(unittest.TestCase):
    identifier = URIRef("rdflib_test")
    dburi = Literal("sqlite://")

    def setUp(self):
        self.store = plugin.get(
            "SQLAlchemy", Store)(identifier=self.identifier, configuration=self.dburi)
        self.graph = ConjunctiveGraph(self.store, identifier=self.identifier)
        self.graph.open(self.dburi, create=True)

    def test_contexts_result(self):
        ctx_id = URIRef('http://example.org/context')
        g = self.graph.get_context(ctx_id)
        g.add((michel, likes, pizza))
        actual = list(self.store.contexts())
        self.assertEqual(actual[0], ctx_id)

So it seems that information about michel liking pizza is transferred in the graph. However, when retrieving the store.contexts() it fails to retrieve the data. Any ideas what might be the cause of this?

I added my commit in the links below.

Bugfix branch issue 100
Bugfix commit

@richardscholtens
Copy link
Contributor

richardscholtens commented Dec 5, 2023

So it does seems to work when the test gets a adjusted a little bit.

def test_contexts_result(self):
    ctx_id = URIRef('http://example.org/context')
    g = self.graph.get_context(ctx_id)
    g.add((michel, likes, pizza))
    actual = list(self.store.contexts(triple=(michel, likes, pizza)))
    self.assertEqual(actual[0], ctx_id)

Here I fill in the parameter triple so it know exactly which triple to look for and only retrieves the contexts it is in. This means it does store the triple somewhere in the contexts. Also see this context function reference

I've also started a pull request.

@mwatts15
Copy link
Collaborator

mwatts15 commented Dec 5, 2023 via email

@richardscholtens
Copy link
Contributor

richardscholtens commented Dec 5, 2023

I believe it has to do with the following code:

# sql.py line 68
        elif select_type == CONTEXT_SELECT:
            select_clause = expression.select(*[table.c.context]).where(whereClause)
        

If I change it to the code below the test test_contexts_results works, but then test_contexts_with_triple fails.

# sql.py line 68
        elif select_type == CONTEXT_SELECT:
            select_clause = expression.select(table.c.context)
            if whereClause:
                select_clause = expression.select(table.c.context).where(whereClause)

New error:

    def __bool__(self):
>       raise TypeError("Boolean value of this clause is not defined")
E       TypeError: Boolean value of this clause is not defined

@richardscholtens
Copy link
Contributor

richardscholtens commented Dec 5, 2023

Ah never mind. Already fixed it with;

if whereClause is not None:

I believe the pull request is ready for reviewing, @mwatts15

@mwatts15
Copy link
Collaborator

mwatts15 commented Dec 7, 2023 via email

@richardscholtens
Copy link
Contributor

Thanks! Let me know if there are any issues.

@richardscholtens
Copy link
Contributor

I noticed I still had an old commit message in the PR. Updated this PR comment.

@richardscholtens
Copy link
Contributor

richardscholtens commented Feb 6, 2024

@mwatts15, I pushed a new fix for one of the bugs. All Postgres test seem to work now. Did not test on the other databases. Could you rerun the pipeline for me?

PR-104

@richardscholtens
Copy link
Contributor

@mwatts15 , I believe this issue can be closed now.

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

No branches or pull requests

3 participants