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

Policy on type annotations #1311

Closed
aucampia opened this issue May 13, 2021 · 1 comment · Fixed by #1407
Closed

Policy on type annotations #1311

aucampia opened this issue May 13, 2021 · 1 comment · Fixed by #1407
Assignees

Comments

@aucampia
Copy link
Member

There are not many changes needed to make to get mypy to pass, and this opens the door to adding more type annotations which makes it easier for users of the library and helps prevents errors.

Are you open to a PR that adds support for mypy to the CI pipelines and makes changes needed in the codebase for it to pass? Any preferences for config and styling (i.e. import typing as t; import typing_extensions as te;)?

Would really make the experience of using rdflib easier IMO.

@nicholascar
Copy link
Member

We have a policy in place:

  • type annotations are required for new PRs
  • devs can run mypy if they like, but we definitely will for CICD testing

This needs to be better documented

@nicholascar nicholascar self-assigned this Jul 12, 2021
aucampia added a commit to aucampia/rdflib that referenced this issue Sep 10, 2021
Fixes RDFLib#1311

Add `mypy` to .drone.yml and fix type errors that come up.

Not type checking examples or tests.

Other changes:

- fix return value for `Graph.serialize` (refs RDFLib#1394)
- remove default value for
  `rdflib.plugins.sparql.algebra.translateAlgebra` (refs RDFLib#1322)
- add .dockerignore to reduce context size and make docker quicker
  to run.
- add .flake8 config to ignore line length as black is managing
  formatting.
- add mypy to docker-compose, makefile and tox.ini
- fix the way csv2rdf is invoked to ensure that the right code gets
  executed.
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 a pull request may close this issue.

2 participants