Skip to content

Constants Addition -- pi, newaxis, nan, inf #18

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

AzeezIsh
Copy link

Addition to _constants.py to reflect Array API Standards. Amended __init.py as well accordingly.

Link reference below:
https://data-apis.org/array-api/2022.12/API_specification/constants.html#objects-in-api

@roaffix
Copy link
Owner

roaffix commented Apr 21, 2024

Is it a spec of 2022?

@AzeezIsh
Copy link
Author

Is it a spec of 2022?

I believe so! The link I provided has "Python array API standard v2022.12" on the header -- https://data-apis.org/array-api/2022.12/API_specification/constants.html#objects-in-api

@roaffix
Copy link
Owner

roaffix commented Apr 23, 2024

We can pass at least black, isort, and flake8 checks. Please, fix the code style.
P.S. you can run locally make pre-commit

@AzeezIsh AzeezIsh force-pushed the pyproject-failures branch from 6e8a98a to c8b8fcf Compare April 23, 2024 21:20
@AzeezIsh
Copy link
Author

We can pass at least black, isort, and flake8 checks. Please, fix the code style. P.S. you can run locally make pre-commit

I just passed them all on the _constants file. Should be good!

@roaffix
Copy link
Owner

roaffix commented Apr 23, 2024

We can pass at least black, isort, and flake8 checks. Please, fix the code style. P.S. you can run locally make pre-commit

I just passed them all on the _constants file. Should be good!

image

but its not good :)

just run locally make pre-commit. We are good if all checks pass

@roaffix
Copy link
Owner

roaffix commented Apr 23, 2024

Quick hack: use black . and isort . These tools will fix the code style automatically, so you don't need to fix it manually

@AzeezIsh
Copy link
Author

AzeezIsh commented Apr 23, 2024

Quick hack: use black . and isort . These tools will fix the code style automatically, so you don't need to fix it manually

make command doesn't work. And i've also confirmed that running isort once again on the local file leads to no changes for _constants.py

_init.py has to be rebased hold on

@roaffix
Copy link
Owner

roaffix commented Apr 23, 2024

Quick hack: use black . and isort . These tools will fix the code style automatically, so you don't need to fix it manually

make command doesn't work. And i've also confirmed that running isort once again on the local file leads to no changes for _constants.py

You could open the failed check and look through the traceback

image

Your commit also affect other files, so the problem can occur not only in constants file

@AzeezIsh
Copy link
Author

Quick hack: use black . and isort . These tools will fix the code style automatically, so you don't need to fix it manually

make command doesn't work. And i've also confirmed that running isort once again on the local file leads to no changes for _constants.py

You could open the failed check and look through the traceback

image

Your commit also affect other files, so the problem can occur not only in constants file

Hopefully it's good now! Mypy is failing in other places that I haven't touched

@roaffix
Copy link
Owner

roaffix commented Apr 23, 2024

what kind of issue with mypy?

@AzeezIsh
Copy link
Author

what kind of issue with mypy?

Bunch of little weird errors, there's a high chance they were already there? It's not required though so the merge should be good

@roaffix
Copy link
Owner

roaffix commented Apr 24, 2024

what kind of issue with mypy?

Bunch of little weird errors, there's a high chance they were already there? It's not required though so the merge should be good

I run master locally and all is good. So, you should also not have any problems with your local machine. Paste a screen of errors here, so I can check if it's a common cross-platform or PR-related issue.

Also, all checks are required TBH, but due to WIP CI status - it is not available on GitHub actions.

@AzeezIsh
Copy link
Author

what kind of issue with mypy?

Bunch of little weird errors, there's a high chance they were already there? It's not required though so the merge should be good

I run master locally and all is good. So, you should also not have any problems with your local machine. Paste a screen of errors here, so I can check if it's a common cross-platform or PR-related issue.

Also, all checks are required TBH, but due to WIP CI status - it is not available on GitHub actions.

(.venv) aishaqui@aishaqui-mobl:~/ArrayFire/arrayfire-py$ black .
Skipping .ipynb files as Jupyter dependencies are not installed.
You can fix this by running pip install "black[jupyter]"
reformatted /home/aishaqui/ArrayFire/arrayfire-py/examples/common/idxio.py
reformatted /home/aishaqui/ArrayFire/arrayfire-py/arrayfire/random.py
reformatted /home/aishaqui/ArrayFire/arrayfire-py/arrayfire/blas.py
reformatted /home/aishaqui/ArrayFire/arrayfire-py/arrayfire/logistic_regression.py
reformatted /home/aishaqui/ArrayFire/arrayfire-py/arrayfire/data.py
reformatted /home/aishaqui/ArrayFire/arrayfire-py/arrayfire/device.py
reformatted /home/aishaqui/ArrayFire/arrayfire-py/examples/machine_learning/mnist_common.py
reformatted /home/aishaqui/ArrayFire/arrayfire-py/arrayfire/arith.py
reformatted /home/aishaqui/ArrayFire/arrayfire-py/examples/machine_learning/logistic_regression.py
reformatted /home/aishaqui/ArrayFire/arrayfire-py/arrayfire/algorithm.py

All done! ✨ 🍰 ✨
10 files reformatted, 63 files left unchanged.
(.venv) aishaqui@aishaqui-mobl:~/ArrayFire/arrayfire-py$ isort .
Fixing /home/aishaqui/ArrayFire/arrayfire-py/arrayfire/logistic_regression.py
Fixing /home/aishaqui/ArrayFire/arrayfire-py/arrayfire/algorithm.py
Fixing /home/aishaqui/ArrayFire/arrayfire-py/arrayfire/random.py
Fixing /home/aishaqui/ArrayFire/arrayfire-py/arrayfire/device.py
Fixing /home/aishaqui/ArrayFire/arrayfire-py/arrayfire/arith.py
Fixing /home/aishaqui/ArrayFire/arrayfire-py/arrayfire/data.py
Fixing /home/aishaqui/ArrayFire/arrayfire-py/arrayfire/blas.py
Fixing /home/aishaqui/ArrayFire/arrayfire-py/examples/machine_learning/logistic_regression.py
Fixing /home/aishaqui/ArrayFire/arrayfire-py/examples/machine_learning/mnist_common.py
Skipped 4 files

(.venv) aishaqui@aishaqui-mobl:~/ArrayFire/arrayfire-py$ mypy .
build/lib/arrayfire/init.py: error: Duplicate module named "arrayfire" (also at "./arrayfire/init.py")
build/lib/arrayfire/init.py: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#mapping-file-paths-to-modules for more info
build/lib/arrayfire/init.py: note: Common resolutions include: a) using --exclude to avoid checking one of them, b) adding __init__.py somewhere, c) using --explicit-package-bases or adjusting MYPYPATH
Found 1 error in 1 file (errors prevented further checking)

@AzeezIsh
Copy link
Author

AzeezIsh commented May 2, 2024

what kind of issue with mypy?

Bunch of little weird errors, there's a high chance they were already there? It's not required though so the merge should be good

I run master locally and all is good. So, you should also not have any problems with your local machine. Paste a screen of errors here, so I can check if it's a common cross-platform or PR-related issue.

Also, all checks are required TBH, but due to WIP CI status - it is not available on GitHub actions.

Just bumping this as I'm working on an ArrayFire API example. My previous merge request also had the same issues with MyPy, so it might just be appearing specific to my local machine. Anything else I could do on my end?

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 this pull request may close these issues.

2 participants