search

RDFLib / sparqlwrapper

A wrapper for a remote SPARQL endpoint
https://sparqlwrapper.readthedocs.io/
Other
526 stars 122 forks source link

Typing #197

Closed eggplants closed 2 years ago

eggplants commented 2 years ago

Fixes: #193

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

aucampia commented 2 years ago

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 https://github.com/RDFLib/rdflib/pull/1683 and then to rdflib.graph

eggplants commented 2 years ago

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 commented 2 years ago

keepalive is a old package to provide HTTP Keep-Alive Handler for urllib2. Currently, this module cannot install due to use_2to3 (https://github.com/wikier/keepalive/issues/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.

nicholascar commented 2 years ago

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 commented 2 years ago

https://github.com/akamhy/waybackpy/issues/151#issuecomment-1033664869

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

nicholascar commented 2 years ago

Great to see all tests passing! I suppose it's just the requirements.txt issues then we're all good.

nicholascar commented 2 years ago

Did you want to put PANDAS into its own requirements.optional.txt as well as the optional install slot in setup.py? Also black in requirements.development.txt?

eggplants commented 2 years ago

Did you want to put PANDAS into its own requirements.optional.txt as well as the optional install slot in setup.py?

Yes.

Also black in requirements.development.txt?

Introducing of Devtools like black, flake8, isort, pytest will do in other PR.

aucampia commented 2 years ago

Could you add mypy to the CI pipeline (i.e. .github/workflows/test.yml)? I worry that if we don't we won't notice future type breakages.

eggplants commented 2 years ago

@aucampia done. 428ce92

aucampia commented 2 years ago

The PR looks good for the most part, but it includes many changes that I think is not needed for type checking to pass, some of them are very clearly correct, others I would have to first analyse, If possible you should keep PRs as limited in scope as possible and I would prefer that the changes which are not needed for type checking to pass to rather be made in separate PRs, see this for more details:

https://github.com/RDFLib/sparqlwrapper/pull/197#discussion_r810545424

I'm going to mark everything I think can go in seperate PRs, I may be wrong about some of them though.

eggplants commented 2 years ago

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

aucampia commented 2 years ago

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 commented 2 years ago

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