Resolving #143

[Abelarm]

Created a function that "install" the latest version of numpy before running the setup, so no code change is needed and no external packages are needed.

The installation of numpy is not completed until the whole setup is complete. (even though you can use it during setup)
If the installation fails the numpy is not installed.

Todo:

• prevent the installation when using with --help
• If merged documentation needs to be updated
• Change the CI

[codecov]

Codecov Report

Merging #202 (fa3d95e) into develop (760eddb) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #202   +/-   ##
========================================
  Coverage    45.47%   45.47%           
========================================
  Files           84       84           
  Lines         9029     9029           
========================================
  Hits          4106     4106           
  Misses        4923     4923           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 760eddb...fa3d95e. Read the comment docs.

[Abelarm]

I am mad at the Codacy Static Code Analysis! otherwise would have been everything green

[dalonsoa]

This is quite neat! Many thanks.

I'm sorting out the Codacy stuff. That import is perfectly legit to check if the package is available so that error should be ignored. You could add # noqa at the end of that line, so it is ignored by the linter, so:

import numpy  # noqa

[Abelarm]

Regarding the three points of the todo,

the first one is really hard, but the other two are easy, should I add them to this PR or separates ones?

[dalonsoa]

I would add them as part of the PR. They are, in the end, consequence of this change, so should be merged together.

Why is the first point so complicated?

[Abelarm]

I would add them as part of the PR. They are, in the end, consequence of this change, so should be merged together.

Why is the first point so complicated?

because the --help is provided by the Setup function of numpy which is not installed...
so probably it's a kind of non-problem because also in the case of running with --help to print the output we need numpy

A kind of side effect

[dalonsoa]

Well, it is undesirable to have numpy installed just for running --help, but I would say it is a minor issue and the benefits outweigh that inconvenience. I would suggest to just ignore that and - if it bothers you - open an issue to sort that out in the future.

[Abelarm]

Cool the test passes also by removing the pip install numpy (so my PR works in all the envs)
should I remove the pip install numpy also from deploy.yml and build_deploy_wheels.yml?

[dalonsoa]

Leave those untouched. Your solution works absolutely fine, but it is specially beneficial for the end users. The ‘deploy’ workflows really affect no one. As they are working fine now, let’s leave them.

[Abelarm]

Leave those untouched. Your solution works absolutely fine, but it is specially beneficial for the end users. The ‘deploy’ workflows really affect no one. As they are working fine now, let’s leave them. On 18 Oct 2021, at 12:13, Luigi Giugliano @.***> wrote:  Cool the test passes also by removing the pip install numpy (so my PR works in all the envs) should I remove the pipi install numpy also from deploy.yml and build_deploy_wheels.yml? — You are receiving this because you commented. Reply to this email directly, view it on GitHub<#202 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABOQHLU3W6MO5MPB2CFO4B3UHP6NHANCNFSM5GC3UW3A.

but it's ok to remove the installation from the tests? or do I revert that commit?

[dalonsoa]

No, no, that's fine. The CI system is there to test that everything works, including the installation process. So it is good that you changed that. As you said, it is good to prove that your solution works in all environments. It is just in the CD system where that is not needed.

[Abelarm]

All the TODOs are done! (the --help left)

Ps: it documented how to generated the docs locally? ( I will use it for the next Issue)

[Abelarm]

@dalonsoa if you are planning to merge it after the end of October could you add the label hacktoberfest-accepted, thank you :)

[dalonsoa]

Actually, I was waiting for you to "request my review", then I'd prove it and merge.