Move tests to root dir. Minor setup refactoring#211
Move tests to root dir. Minor setup refactoring#211syurkevi merged 1 commit intoarrayfire:masterfrom
Conversation
| @@ -1,4 +1,4 @@ | |||
| #!/usr/bin/python | |||
| #!/usr/bin/env python | |||
requirements.txt
Outdated
| @@ -0,0 +1,6 @@ | |||
| setuptools==40.6.2 | |||
| numba==0.45.1 | |||
| numpy==1.17.1 | |||
There was a problem hiding this comment.
I would like to move toward pipenv. I don't like these pinned dependencies. How did you determine the versions? Are they minimum supported. pipenv would solve all of these.
| num_args = len(sys.argv) | ||
|
|
||
| if (num_args > 1): | ||
| if num_args > 1: |
| print_func(a.is_real_floating(), a.is_floating(), a.is_integer(), a.is_bool()) | ||
|
|
||
| a = af.Array(host.array('I', [7, 8, 9] * 3), (3,3)) | ||
| a = af.Array(host.array("I", [7, 8, 9] * 3), (3, 3)) |
There was a problem hiding this comment.
Subjective: I dislike double quotes for a "char"-like string.
There was a problem hiding this comment.
If AF lib won't be extended with modules for web-dev, so it's okay to change all quotes to single and in another case, it'd be better to use double. But what's the point of quotes mixing? It decreases readability and adds chaos a bit into a code style.
|
Moving the tests to the root directory is a good move. Once we figure out the pipenv this should be good to merge. |
|
TBH I'm not sure if requirements should even be added in this PR. The feature with binaries link (see added TODO by original maintainer) will require an update for all python wrapper dependencies. But it was added just to follow recommendations for python project structure. So, I can remove them at all for now and the project will still work just fine. |
|
Everything looks good to me. I agree that the requirements.txt file is not required because its mostly for tests and interop functionality. |
roaffix
force-pushed
the
feature/move-tests-to-rootdir
branch
from
f27f419 to
632cb8f
Compare
|
Removed |
3 participants
IMO, there is no purpose to keep and deliver
tests/within the package. It has no use cases here and it makes pkg even dirtier than it is now. So, I made a small PR that moves outtests/to the root directory. There are minor changes were made according to thePEP-8.I added missing
requirements.txtfile and added minor changes tosetup.py.Also, I think it would be better to extend
max-line-lengthfrom default 79 symbols to 119 symbols because your API requires a lot of arguments that make functions too long. 119 symbols will decrease the extra amount of lines in the future.