-
Notifications
You must be signed in to change notification settings - Fork 6
Fix install #2
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
base: master
Are you sure you want to change the base?
Fix install #2
Conversation
| **Python Libraries** | ||
| * [scikit-learn] >=0.19.0 | ||
| * [scikit-learn] >=0.19.0, <= 0.20 | ||
| * [Numpy] >= 1.15 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
forgot to update the dependency version on numpy, the last numpy release to support Python 2.7: see https://docs.scipy.org/doc/numpy/release.html#numpy-1-16-0-release-notes
should be: [Numpy] <= 1.16
jos4uke
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please can you review my comments, I forgot to add some doc and useful comments so people are aware of some mandatory step to make install work. Thanks in advance
|
|
||
|
|
||
| ## Test run | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well I was not able to run this test because of the missing input data.
If you can add a link where we can download them it would be great.
Thanks in advance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The input data link has been added in the readme section. Could you please double check?
| **Python Libraries** | ||
| * [scikit-learn] >=0.19.0 | ||
| * [scikit-learn] >=0.19.0, <= 0.20 | ||
| * [Numpy] >= 1.15 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you update it to: [Numpy] <= 1.16
| $ python setup.py | ||
| $ python setup.py install | ||
|
|
||
| # clean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should have mentioned this clean command is only after successful package install and optional
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you edit the README and add this comment plz
|
|
||
| ``` | ||
|
|
||
| ## Installation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I forgot to tell I updated the setup.py file to set 2 variables INCPATH and LIBPATH respectively to the include and lib path to gsl. And the user should edit them to make it work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you plz update the README and add these instructions so people are aware of this mandatory step
|
|
||
| INCPATH="/opt/share/FLOCAD/userspace/jtran1/miniconda3/envs/sci_env/include" | ||
| LIBPATH="/opt/share/FLOCAD/userspace/jtran1/miniconda3/envs/sci_env/lib" | ||
| LIBS="-lgsl -lgslcblas -lpthread -L{}".format(LIBPATH) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should add a comment to tell people to edit these 2 variables INCPATH and LIBPATH to point to their include and lib path to gsl
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you plz add such a comment above these 2 variables
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And maybe replace my paths by some placeholder strings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jos4uke I am trying to push some changes to the branch. May you give me permission to do so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes for sure
Hi
thanks for this great work you have done on this piece of software.
I sent you some fixes to the current install doc, because I encountered some troubles with the LINE compilation due to the missing path to my own lib and inc where I installed gsl. And also fixed requirements versions not supporting Python 2.7 as numpy, scipy and scikit-learn packages.
And finally fixed the install python setup.py command with missing install target command.
I updated the README for more details.
Best