Move tox to pytest

[alejandrosame]

Description

Fixes #137 and #192. Apart from substituting tox with pytest testing, Makefile and Github workflow files have been aligned for consitency. Since right now, we lack meaningful Python code, coverage dependency and code has been deleted also for simplicity.

Affected Dependencies

Tox deleted. Coverage deleted.

How has this been tested?

• Locally, I ran make test.
• Github Actions were tested on a mock PR: Fix/tox to pytest check PR alejandrosame/PyDP#3

Checklist

• I have followed the Contribution Guidelines and Code of Conduct
• I have commented my code following the OpenMined Styleguide
• I have labeled this PR with the relevant Type labels
• My changes are covered by tests

[chinmayshah99]

@alejandrosame the tests are failing.
Also, is this tested on a system with conda installed?

[alejandrosame]

@chinmayshah99 If you check the log, the error is related to the current bug we have with bounds. If you want, I can already deactivate the tests for bounded methods in this PR since it is more or less a related issue (having consistent tests ASAP).

Regarding conda, they have not been tested using conda. As the changes show, pipenv was added to Github actions for consistency with Docker and host execution. I think validating conda support should be a separate issue/PR (it will require also more documentation updates, new github actions, etc).

[chinmayshah99]

Validating with conda is a must, since a lot of our folks use conda. Regards Chinmay Shah

…

On Mon, Jul 13, 2020, 16:37 Alejandro Sánchez Medina < ***@***.***> wrote: @chinmayshah99 <https://github.com/chinmayshah99> If you check the log, the error is related to the current bug we have with bounds. If you want, I can already deactivate the tests for bounded methods in this PR if you want since it is more or less related. Regarding conda, they have not been tested using conda. As the changes show, pipenv was added to Github actions for consistency with Docker and host execution. I think validating conda support should be a separate issue/PR (it will require also more documentation updates, new github actions, etc). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#197 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACTTOFJ6RU4EJ3CY3HHRPTLR3LTGXANCNFSM4OYAJKHQ> .

[chinmayshah99]

Is this needed now that we have removed pycodestyle?

[alejandrosame]

Indeed it is not needed since we introduced Black. I'll delete the section.

[chinmayshah99]

Error when running make test

cs@cs-VirtualBox:~/Desktop/PyDP$ make test
pipenv run black ./ --check --diff
All done! ✨ 🍰 ✨
22 files would be left unchanged.
pipenv run ./run-clang-format.py -r src/bindings/
Traceback (most recent call last):
  File "/home/cs/.local/bin/pipenv", line 8, in <module>
    sys.exit(cli())
  File "/home/cs/.local/lib/python3.6/site-packages/pipenv/vendor/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/home/cs/.local/lib/python3.6/site-packages/pipenv/vendor/click/core.py", line 696, in main
    _verify_python3_env()
  File "/home/cs/.local/lib/python3.6/site-packages/pipenv/vendor/click/_unicodefun.py", line 124, in _verify_python3_env
    ' mitigation steps.' + extra
RuntimeError: Click will abort further execution because Python 3 was configured to use ASCII as encoding for the environment. Consult https://click.palletsprojects.com/en/7.x/python3/ for mitigation steps.

This system supports the C.UTF-8 locale which is recommended.
You might be able to resolve your issue by exporting the
following environment variables:

    export LC_ALL=C.UTF-8
    export LANG=C.UTF-8
Makefile:56: recipe for target 'check-style-cpp' failed
make: *** [check-style-cpp] Error 1

Though I can add these environment variables, this error is not desirable.

[alejandrosame]

Error when running make test

cs@cs-VirtualBox:~/Desktop/PyDP$ make test
pipenv run black ./ --check --diff
All done! ✨ 🍰 ✨
22 files would be left unchanged.
pipenv run ./run-clang-format.py -r src/bindings/
Traceback (most recent call last):
  File "/home/cs/.local/bin/pipenv", line 8, in <module>
    sys.exit(cli())
  File "/home/cs/.local/lib/python3.6/site-packages/pipenv/vendor/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/home/cs/.local/lib/python3.6/site-packages/pipenv/vendor/click/core.py", line 696, in main
    _verify_python3_env()
  File "/home/cs/.local/lib/python3.6/site-packages/pipenv/vendor/click/_unicodefun.py", line 124, in _verify_python3_env
    ' mitigation steps.' + extra
RuntimeError: Click will abort further execution because Python 3 was configured to use ASCII as encoding for the environment. Consult https://click.palletsprojects.com/en/7.x/python3/ for mitigation steps.

This system supports the C.UTF-8 locale which is recommended.
You might be able to resolve your issue by exporting the
following environment variables:

    export LC_ALL=C.UTF-8
    export LANG=C.UTF-8
Makefile:56: recipe for target 'check-style-cpp' failed
make: *** [check-style-cpp] Error 1

Though I can add these environment variables, this error is not desirable.

I added an ENV file so Pipenv automatically loads those variables inside the virtualenv. That should fix the issue. Can you please confirm that it fixes the execution of make test in your system?

[chinmayshah99]

I added an ENV file so Pipenv automatically loads those variables inside the virtualenv. That should fix the issue. Can you please confirm that it fixes the execution of make test in your system?

This works well! Just to confirm, there is no-inplace editing right? The code does not automatically re-format the correct code even though it suggests what needs to be done. Can we add that if it's possible?

[alejandrosame]

I added an ENV file so Pipenv automatically loads those variables inside the virtualenv. That should fix the issue. Can you please confirm that it fixes the execution of make test in your system?

This works well! Just to confirm, there is no-inplace editing right? The code does not automatically re-format the correct code even though it suggests what needs to be done. Can we add that if it's possible?

I wouldn't overload make test with in-place editing. In my opinion, the easiest workflow is to hook the relevant formatting functions to the editor, so the code is formatted every time is saved. That's helpful because one gets to see the actual expected formatting in the editor, and, in my opinion, that helps getting used to the style. Therefore, I think we should notify this to contributors, so they know that possibility exists.

Apart from that, for those that can't arrange the editor (or wouldn't care because, for example, they just want to quickly make a one time contribution) we could provide new formatting actions. That way, make test can announce that they should run, for example, make format-code to automatically format the faulty files.

What do you think?

[chinmayshah99]

Apart from that, for those that can't arrange the editor (or wouldn't care because, for example, they just want to quickly make a one time contribution) we could provide new formatting actions. That way, make test can announce that they should run, for example, make format-code to automatically format the faulty files.

What do you think?

Yeah, we can add that!

Also can we also add make build that just builds/ run the ./build_pydp.sh?

[alejandrosame]

Yeah, we can add that!

Also can we also add make build that just builds/ run the ./build_pydp.sh?

Done on the last 2 commits.

[chinmayshah99]

LGTM! Need to confirm if all tests run perfectly :)