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

Typing #197

Merged
merged 39 commits into from
Feb 21, 2022
Merged

Typing #197

merged 39 commits into from
Feb 21, 2022

Conversation

eggplants
Copy link
Contributor

@eggplants eggplants commented Jan 24, 2022

Fixes: #193

I can't wait for the type rdflib.graph.{Graph, ConjunctiveGraph} to be provided...

@aucampia
Copy link
Member

I can't wait for the type rdflib.graph.{Graph, ConjunctiveGraph} to be provided...

Just a reminder, reviews are open to everyone, I will get to rdflib.store after RDFLib/rdflib#1683 and then to rdflib.graph

@eggplants
Copy link
Contributor Author

eggplants commented Jan 25, 2022

12 ignored errors should be fixed in future:

$ grep -rA4 '# type: ignore'
SPARQLWrapper/Wrapper.py:        retval.parse(self.response, format="xml")  # type: ignore[no-untyped-call]
SPARQLWrapper/Wrapper.py-        r_retval = cast(Graph, retval)
SPARQLWrapper/Wrapper.py-        return r_retval
SPARQLWrapper/Wrapper.py-
SPARQLWrapper/Wrapper.py-    def _convertN3(self) -> bytes:
--
SPARQLWrapper/Wrapper.py:        retval.parse(self.response, format="json-ld")  # type: ignore[no-untyped-call]
SPARQLWrapper/Wrapper.py-        r_retval = cast(Graph, retval)
SPARQLWrapper/Wrapper.py-        return r_retval
SPARQLWrapper/Wrapper.py-
SPARQLWrapper/Wrapper.py-    def convert(self) -> ConvertResult:
--
SPARQLWrapper/Wrapper.py:            from keepalive import HTTPHandler  # type: ignore[import]
SPARQLWrapper/Wrapper.py-
SPARQLWrapper/Wrapper.py:            if urllib.request._opener and any(  # type: ignore[attr-defined]
SPARQLWrapper/Wrapper.py:                isinstance(h, HTTPHandler) for h in urllib.request._opener.handlers  # type: ignore[attr-defined]
SPARQLWrapper/Wrapper.py-            ):
SPARQLWrapper/Wrapper.py-                # already installed
SPARQLWrapper/Wrapper.py-                return
SPARQLWrapper/Wrapper.py-
--
SPARQLWrapper/SmartWrapper.py:    def query(self) -> Union[Bindings, QueryResult]:  # type: ignore[override]
SPARQLWrapper/SmartWrapper.py-        """
SPARQLWrapper/SmartWrapper.py-        Execute the query and do an automatic conversion.
SPARQLWrapper/SmartWrapper.py-
SPARQLWrapper/SmartWrapper.py-        Exceptions can be raised if either the URI is wrong or the HTTP sends back an error.
--
SPARQLWrapper/SmartWrapper.py:    def queryAndConvert(  # type: ignore[override]
SPARQLWrapper/SmartWrapper.py-        self,
SPARQLWrapper/SmartWrapper.py-    ) -> Union[Union[Bindings, QueryResult], QueryResult.ConvertResult]:
SPARQLWrapper/SmartWrapper.py-        """This is here to override the inherited method; it is equivalent to :class:`query`.
SPARQLWrapper/SmartWrapper.py-
--
SPARQLWrapper/sparql_dataframe.py:            vv = rdflib.term.Literal(v.value, datatype=v.datatype).toPython()  # type: ignore[no-untyped-call]
SPARQLWrapper/sparql_dataframe.py-            row[k] = vv
SPARQLWrapper/sparql_dataframe.py-        d.append(row)
SPARQLWrapper/sparql_dataframe.py-    return d
SPARQLWrapper/sparql_dataframe.py-
--
build/lib/SPARQLWrapper/Wrapper.py:        retval.parse(self.response, format="xml")  # type: ignore[no-untyped-call]
build/lib/SPARQLWrapper/Wrapper.py-        r_retval = cast(Graph, retval)
build/lib/SPARQLWrapper/Wrapper.py-        return r_retval
build/lib/SPARQLWrapper/Wrapper.py-
build/lib/SPARQLWrapper/Wrapper.py-    def _convertN3(self) -> bytes:
--
build/lib/SPARQLWrapper/Wrapper.py:        retval.parse(self.response, format="json-ld")  # type: ignore[no-untyped-call]
build/lib/SPARQLWrapper/Wrapper.py-        r_retval = cast(Graph, retval)
build/lib/SPARQLWrapper/Wrapper.py-        return r_retval
build/lib/SPARQLWrapper/Wrapper.py-
build/lib/SPARQLWrapper/Wrapper.py-    def convert(self) -> ConvertResult:
--
build/lib/SPARQLWrapper/Wrapper.py:            from keepalive import HTTPHandler  # type: ignore[import]
build/lib/SPARQLWrapper/Wrapper.py-
build/lib/SPARQLWrapper/Wrapper.py:            if urllib.request._opener and any(  # type: ignore[attr-defined]
build/lib/SPARQLWrapper/Wrapper.py:                isinstance(h, HTTPHandler) for h in urllib.request._opener.handlers  # type: ignore[attr-defined]
build/lib/SPARQLWrapper/Wrapper.py-            ):
build/lib/SPARQLWrapper/Wrapper.py-                # already installed
build/lib/SPARQLWrapper/Wrapper.py-                return
build/lib/SPARQLWrapper/Wrapper.py-
--
build/lib/SPARQLWrapper/SmartWrapper.py:    def query(self) -> Union[Bindings, QueryResult]:  # type: ignore[override]
build/lib/SPARQLWrapper/SmartWrapper.py-        """
build/lib/SPARQLWrapper/SmartWrapper.py-        Execute the query and do an automatic conversion.
build/lib/SPARQLWrapper/SmartWrapper.py-
build/lib/SPARQLWrapper/SmartWrapper.py-        Exceptions can be raised if either the URI is wrong or the HTTP sends back an error.
--
build/lib/SPARQLWrapper/SmartWrapper.py:    def queryAndConvert(  # type: ignore[override]
build/lib/SPARQLWrapper/SmartWrapper.py-        self,
build/lib/SPARQLWrapper/SmartWrapper.py-    ) -> Union[Union[Bindings, QueryResult], QueryResult.ConvertResult]:
build/lib/SPARQLWrapper/SmartWrapper.py-        """This is here to override the inherited method; it is equivalent to :class:`query`.
build/lib/SPARQLWrapper/SmartWrapper.py-
--
build/lib/SPARQLWrapper/sparql_dataframe.py:            vv = rdflib.term.Literal(v.value, datatype=v.datatype).toPython()  # type: ignore[no-untyped-call]
build/lib/SPARQLWrapper/sparql_dataframe.py-            row[k] = vv
build/lib/SPARQLWrapper/sparql_dataframe.py-        d.append(row)
build/lib/SPARQLWrapper/sparql_dataframe.py-    return d
build/lib/SPARQLWrapper/sparql_dataframe.py-

@eggplants
Copy link
Contributor Author

keepalive is a old package to provide HTTP Keep-Alive Handler for urllib2. Currently, this module cannot install due to use_2to3 (wikier/keepalive#10)
At this point in time, we have dropped support for Python 2 and old Python 3.x. So I think it's no problem to drop keepalive.

@eggplants eggplants marked this pull request as ready for review January 27, 2022 11:28
@nicholascar
Copy link
Member

I've merged the conflicting updated .cfg file: @eggplants can you just confirm that the merged content in the file is correct, i.e. that all sections in it should be there after your other PR has now been merged?

@eggplants
Copy link
Contributor Author

eggplants commented Feb 17, 2022

akamhy/waybackpy#151 (comment)

I am making this same silly mistake. I'm busy now, so I'll fix it tonight. I'm so sorry.

setup.cfg Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
setup.cfg Show resolved Hide resolved
SPARQLWrapper/Wrapper.py Outdated Show resolved Hide resolved
SPARQLWrapper/Wrapper.py Outdated Show resolved Hide resolved
SPARQLWrapper/Wrapper.py Outdated Show resolved Hide resolved
SPARQLWrapper/Wrapper.py Outdated Show resolved Hide resolved
SPARQLWrapper/Wrapper.py Outdated Show resolved Hide resolved
SPARQLWrapper/Wrapper.py Outdated Show resolved Hide resolved
SPARQLWrapper/Wrapper.py Outdated Show resolved Hide resolved
SPARQLWrapper/main.py Outdated Show resolved Hide resolved
@eggplants
Copy link
Contributor Author

eggplants commented Feb 20, 2022

I reverted changes potentially break behavior. But they all are needed for strict type checking. So mypy test is failed without them.

@aucampia
Copy link
Member

I reverted changes potentially break behavior. But they all are needed for strict type checking. So mypy test is failed without them.

I will make it work, will make some commits to your branch

@eggplants
Copy link
Contributor Author

Do what you want and merge. I'm tired. Sorry.

Copy link
Member

@aucampia aucampia left a comment

Choose a reason for hiding this comment

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

@eggplants thanks for the PR and patience.

I made this a bit more terse and split of some of the changes to other PRs:

There are potentially more changes that were in the PR that could be split of, some of the code in this library needs some reconsideration, like many cases of try: ... except: pass - but best to do that separately.

There is still a couple of things in this PR that are not entirely essential for it's scope, like changes to docstrings and the change in imports, but they seem safe enough.

Copy link
Member

@nicholascar nicholascar left a comment

Choose a reason for hiding this comment

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

@eggplants thanks for your large effort here and for responding to the very many review resuests!

@aucampia thanks for all your reviews and the additions you've made too!

@nicholascar nicholascar merged commit 3f12d4b into RDFLib:master Feb 21, 2022
@eggplants eggplants deleted the typing branch February 21, 2022 10:20
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.

Typing
3 participants