Added PEP 561 compliance (#945) #946
Conversation
|
The one travis-ci failed run seems to be unrelated to my additions as far as I'm aware |
mfeurer
left a comment
mfeurer
left a comment
There was a problem hiding this comment.
Thanks a lot for the prompt PR and for contributing to our repository! This looks good, but unfortunately it does not quite work yet - please see my comments inline.
| # Assumes mypy relies solely on the PEP 561 standard | ||
| # Also assumes mypy to be available | ||
| if ! python -m mypy -c "import openml"; then | ||
| echo "Failed: PEP 561 compliance" |
There was a problem hiding this comment.
Could you please make this more strict, i.e. fail the installation?
ci_scripts/install.sh
Outdated
|
|
||
| # PEP 561 compliance check | ||
| # Assumes mypy relies solely on the PEP 561 standard | ||
| # Also assumes mypy to be available |
There was a problem hiding this comment.
Unfortunately, we don't install mypy and the test therefore fails. An easy fix for now would be to install mypy here as well.
There was a problem hiding this comment.
You can see this in the travis-ci log for the different builds if you expand the entry for install.sh:
/home/travis/miniconda/envs/testenv/bin/python: No module named mypy
Failed: PEP 561 compliance
|
Thanks for the quick review! I'll have a go with it again tomorrow :) Is there a specific way to fail the build or should I just give an |
|
|
|
The logs seem to indicate that it passed the mypy check now :) |
|
Thank you very much for your contribution! |
2 participants
Reference Issue
#945
What does this PR implement/fix? Explain your changes.
Implements the PEP 561 standard for Distributing and Packaging Type Information.
How should this PR be tested?
By using
mypy -c 'import openml'mypyshould succeed and report no errorsAny other comments?
I'm not sure how the test result should be reported.
For now it's a simple if/else print statement in
ci_scripts/install.shbut please point me in the right direction otherwise.I'm not even sure if the
ci_scripts/install.shscript ran during any of the testing steps described in CONTRIBUTING.md but there were no errors. I tried to run it manually by it seems to be intended to run from a CI environment.As
setup.pydoes not installmypy, I also did a manual test bypip install mypyfollowed bymypy -c 'import openml'which succeeded.I can also verify that it ran on the locally installed package by running
Lastly, as the documentation says nothing about
py.typedcontents and is assumed blank, I did not add the BSD3 clause to it.