[WIP] MAINT: robustify install for different platforms
I haven't tested it properly but I think this would make the installation process more robust
also see: https://github.com/jonescompneurolab/hnn-core/discussions/556#discussioncomment-3491073
Codecov Report
Merging #560 (8cef839) into master (3597490) will decrease coverage by
0.05%. The diff coverage is92.30%.
:exclamation: Current head 8cef839 differs from pull request most recent head 1d00098. Consider uploading reports for the commit 1d00098 to get more accurate results
@@ Coverage Diff @@
## master #560 +/- ##
==========================================
- Coverage 91.88% 91.84% -0.05%
==========================================
Files 22 22
Lines 4252 4253 +1
==========================================
- Hits 3907 3906 -1
- Misses 345 347 +2
| Impacted Files | Coverage Δ | |
|---|---|---|
| hnn_core/network_builder.py | 94.51% <92.30%> (+0.66%) |
:arrow_up: |
| hnn_core/parallel_backends.py | 81.11% <0.00%> (-1.12%) |
:arrow_down: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
How would we go about testing this?
I guess two steps:
$ python setup.py sdist
Then go inside the archived folder (which is what gets uploaded to pip) and run:
$ pip install .
on different machines ... I guess that will simulate the installation process. You'd want to do this in a fresh conda environment of course.
It won't work for (pure) Windows because the first step that requires installation of Neuron will fail ... that needs a separate fix.
I'll try to give it a shot later this week. Good luck tomorrow Mainak!
@rythorpe touching base on this. Did we manage to check? I read today in some online forums that mknrndll is the command on windows not nrnivmodl. Can you confirm?
@rythorpe touching base on this. Did we manage to check? I read today in some online forums that
mknrndllis the command on windows notnrnivmodl. Can you confirm?
I haven't yet gotten a chance to test this on Windows but will look into it as soon as I can. Hopefully I'll be able to do this later this week.
@jasmainak I was able to get a native Windows installation to work using a source distribution built off this branch! To get this to work, I had to:
- install NEURON at root (it doesn't need to be installed inside the python environment's lib/sit-packages). Somehow the python NEURON library is magically found by any open python shell.
- using the Anaconda shell, create/activate an environment with all the hnn-core dependencies except NEURON.
- after removing the NEURON dependency in
setup.pyandpyproject.toml, build a source distribution of hnn-core.
Note that I never tried using the mknrndll command.
@rythorpe I don't think we want to recommend users to build the distribution. It should literally work with pip install hnn_core. Did you try pip install . or did you run a different command? Removing the automatic install of NEURON is fine by me if it helps making Windows install smoother ...
Somehow the python NEURON library is magically found by any open python shell.
This will likely cause us headaches in the future ... if the mod files are compiled in one version of Python, it may fail in the other environment with a different version of Python.
Did you check that you are able to run an hnn-core example?
I'm guessing they set the PYTHONPATH to the neuron install directory? That might be the black magic which let's neuron work. One option for more control is to provide an environment_windows.yml file:
https://docs.conda.io/projects/conda/en/latest/user-guide/tasks/manage-environments.html#setting-environment-variables
The environment_windows.yml file should contain PYTHONPATH that points to the neuron directory. Otherwise the PYTHONPATH should be unset
When I say I used a source distribution, I meant that I first built the source distribution and then called pip install . from inside the build directory in order to simulate the process of pip install hnn_core. I'm not suggesting that others should use this exact method to install hnn_core.
I'm worried about relying to heavily on environment variables as well. I tested for this with echo $PYTHONPATH, but nothing returned. I'm also probably doing it wrong in Windows....
How would we provide an environment_windows.yml file that sets PYTHONPATH and is not dependent on the exact location the user decided to install NEURON?
pip install . should build as well as copy the built files to anaconda directory ... you shouldn't have to build separately ... and were you able to run the examples?
pip install .should build as well as copy the built files to anaconda directory ... you shouldn't have to build separately ...
Then why do we need to run setup.py when doing a development build? I thought we needed to compile the mod files first, which is why you instructed me to do a source build and then run pip install above?
and were you able to run the examples?
Yep, I ran the evoked example.
Then why do we need to run setup.py when doing a development build? I thought we needed to compile the mod files first, which is why you instructed me to do a source build and then run pip install https://github.com/jonescompneurolab/hnn-core/pull/560#issuecomment-1229362241?
A regular pip install (which internally calls python setup.py install) consists of two steps: a) first build the mod files, then b) copy the built dll and python files to the anaconda directory. Now the way editable install works is that it merely puts a symbolic link to the local python files in the anaconda directory, effectively skipping a). See also:
https://stackoverflow.com/questions/51243633/python-setuptools-setup-py-install-does-not-automatically-call-build
We need to test what the user would do (a regular pip install), not what the developer would do (build mod files + symbolic link)
@rythorpe can you share exactly the commands you executed? Maybe we are confusing terminology here ... did you run:
$ python setup.py sdist
or:
$ python setup.py build_py
I thought I had followed your directions above. Maybe I misunderstood though:
$ python setup.py sdist
$ cd <dist_directory>
$ pip install .
okay, sorry for the noise. I misunderstood what you meant by "built a source distribution" ... sdist only creates an archive
Do you feel confident enough to recommend this as the main install method? Does the new GUI also work fine?
okay, sorry for the noise. I misunderstood what you meant by "built a source distribution" ... sdist only creates an archive
Oh my bad. I though "sdist" was shorthand for "source distribution".
Do you feel confident enough to recommend this as the main install method? Does the new GUI also work fine?
Kind of. I feel confident that it'll work with the current versions of Windows10 + NEURON + hnn_core, however, I'm still worried about us officially supporting a Windows installation. That fact that we can't eliminate the dependency on sketchy environment variables and rely on a manual installation of NEURON gives me consternation.
I haven't tried it with the GUI yet, but I did test out the hnn_core evoked example using ipython. Assuming the ipywigdets works the same in Windows as it does for Linux, we should be good. I can give it another try sometime this week though just to make sure.
I just pushed a commit that should not install Neuron automatically on Windows: https://github.com/jonescompneurolab/hnn-core/pull/560/commits/cd568788c5b8bbdb076c9ba183b0c3f4dc1bc4c6
I think we should be clear in the docs that Neuron install problems should go to Neuron forums, it's not our responsibility. If Neuron is installed, then hopefully hnn-core install "just works" presuming the user is not trying to use multiple Neuron versions in different environments.
Now the only issue is that we don't have Windows CIs to catch quirks in the code for Windows (e.g., how paths are handled etc)
Okay I started adding a Windows CI while I was at it ... feel free to take over though if this is a priority for you. I may not be able to properly debug until next week.
alright guys, this bad boy is good to go. I'll call it a productive sprint if this works ;-)
@jasmainak is there a way to increase the timeout on CircleCI from this PR? This doc build on the master branch is failing since something is taking >10 min to run. If I have time I'll try and dig and see what the culprit is.
we could using this: https://support.circleci.com/hc/en-us/articles/360007188574-Build-has-Hit-Timeout-Limit
But probably the build process needs to be fixed instead. The GUI example is taking too long to build @chenghuzi . Could there be some outputs during the build?
Removed mne from the CIs. Good to go from my end!
Looks good, I'm going to go ahead and merge. Thanks @jasmainak!
So barring the last couple of GUI fixes, we are ready for release?!