Skip to content
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

elephant dir packages are back #239

Merged
merged 9 commits into from Jul 21, 2019

Conversation

dizcza
Copy link
Member

@dizcza dizcza commented Jul 10, 2019

Fixes #237: dir(elephant) should contain its internal packages.
Current travis release parses "- python -c '<python command>'" lines in .travis.yml correctly, however it still does not download fim.so module during the pip install .


Old comment. Not actual anymore (see updates 1 & 2).

That's why I created tools folder for setup package maintenance and moved fim_manager.py there. tools dir is not included in elephant wheel tarball.


Update 1.
I removed tools folder, because we should not ship it with elephant, and moved downloading fim.so in setup.py under the if sys.argv[1] != 'sdist' condition. It means Travis builds won't have fim installed. I also skipped time-consuming test_spade_cpp when we don't have fim.so since this test takes >10 min and causes Travis to hang.


Update 2.
python setup.py install now downloads fim.so in Travis.
Keep in mind, nosetests --cover-package=elephant is run in the project root folder and actually, travis never uses elephant from its lib/site-packages/elephant/, because it finds elephant module in the current project dir. So I included python setup.py install only to download fim.so.


Outdated. See Updates 1 & 2.
I want to explain what is happening behind the travis code:

  - python -c "from tools.fim_manager import download_spade_fim; download_spade_fim()"
  - pip install .
  - python -c "from elephant.spade import HAVE_FIM; assert HAVE_FIM"
  1. download fim.so to elephant/spade_src/fim.so
  2. pip install . copies downloaded fim.so file into the site-packages folder, because there is a line in MANIFEST.in recursive-include elephant/spade_src *.so *.pyd
  3. assert that fim is available (the build will fail if assert is triggered)

@coveralls
Copy link
Collaborator

coveralls commented Jul 10, 2019

Coverage Status

Coverage decreased (-0.02%) to 70.954% when pulling 11ca2e8 on INM-6:elephant_dir_packages into 3c30574 on NeuralEnsemble:master.

@dizcza dizcza requested a review from mdenker July 10, 2019 16:19
@mdenker mdenker added the bug Indicates an unexpected problem or unintended behavior label Jul 12, 2019
elephant/__init__.py Outdated Show resolved Hide resolved
MANIFEST.in Show resolved Hide resolved
…at setup while macking a tarball with 'sdist' command

travis not fixed downloading fim
@dizcza
Copy link
Member Author

dizcza commented Jul 18, 2019

Ready to merge.

Copy link
Member

@mdenker mdenker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pip install . still failed for me regarding fim.so, and with several files missing from the installed packaged. I think this should be fixed by the two inline comments. My complete MANIFEST.in read

include requirements*.txt
include README.rst
include LICENSE.txt
include AUTHORS.txt
recursive-include elephant *.py
include elephant/VERSION
include elephant/current_source_density_src/README.md
include elephant/current_source_density_src/test_data.mat
include elephant/spade_src/LICENSE
recursive-include elephant/spade_src *.so *.pyd
recursive-include tools *.py
include elephant/test/spike_extraction_test_data.txt
recursive-include doc *
prune doc/_build

MANIFEST.in Show resolved Hide resolved
MANIFEST.in Show resolved Hide resolved
@dizcza
Copy link
Member Author

dizcza commented Jul 19, 2019

pip install . still failed for me regarding fim.so

How did you check it? Are you sure you've updated elephant_dir_packages branch locally? Did you leave your project source dir once you installed elephant package with pip install .? Because when you run pip install . and then python -c "from elephant.spade import HAVE_FIM; print(HAVE_FIM)" inside the same folder it tries to find fim.so in your source project dir rather than in .../lib/python3.7/site-packages/elephant/.
Just to be sure, cd ~ each time after you run pip install ..

and with several files missing from the installed packaged

Please double check. If you use conda, navigate to ~/anaconda3/envs/<env-name>/lib/python3.7/site-packages/elephant and check the files there. There should be all .py files and fim.so in spade_src folder.

P.S. I just rerun pip install . and it works as expected: all .py are preserved (even though I removed recursive-include elephant *.py line) and fim.so in downloaded.

@mdenker
Copy link
Member

mdenker commented Jul 19, 2019

True, I think I made some mistake regarding fim.so (maybe I forgot to cd, that could be), however, regarding the second point, the .py files in spade_src and current_source_density_src are definitely missing in the site-packages folder.

My local branch is updated to 8c41e65.

ls ~/miniconda3/envs/tttt/lib/python3.7/site-packages/elephant/current_source_density_src/
README.md  test_data.mat

(Also, due to this, import elephant fails, since it is missing the KCSD modules.)

@dizcza
Copy link
Member Author

dizcza commented Jul 19, 2019

the .py files in spade_src and current_source_density_src are definitely missing in the site-packages folder

This is strange because I do see these files in my conda env when I do a fresh pip install .. Maybe, once again, it's the issue of pip caching policy about which I'm not aware of.
Anyway, I put recursive-include elephant *.py line back in the manifest. Could you confirm it works now for you? Both

  • pip install .
  • python setup.py sdist ; pip install dist/elephant-0.6.2.tar.gz

Copy link
Member

@mdenker mdenker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, with this latest line, it works. Strange, indeed. My pip version is 19.1.1, not sure if this has something to do with it. Anyhow, I think we tested all possibilities by now.
Checking on the setup.py commands, I think sdist is the only one we need to consider. When using bdist for a binary distribution, I guess we "do" want to have the binaries included as well. For me, this is good to go now.

@mdenker mdenker merged commit c013e39 into NeuralEnsemble:master Jul 21, 2019
dizcza added a commit to INM-6/elephant that referenced this pull request Aug 1, 2019
commit 19d3c7b
Author: Danylo Ulianych <dizcza@gmail.com>
Date:   Thu Aug 1 15:09:34 2019 +0200

    fixed min requirements (NeuralEnsemble#235)

    * fixed min requirements

    * travis install libopenmpi-dev

    * added .coveragerc omit=elephant/test* rule not to count test files as source code

commit 36e6096
Author: Danylo Ulianych <dizcza@gmail.com>
Date:   Wed Jul 31 16:55:47 2019 +0200

    Integrated GPFA (NeuralEnsemble#233)

    * Rename neural_trajectory to gpfa and refactor the codes accordingly

    * Remove codes related to the (not fully implemented) cross-validation feature

    * added tqdm as an extra requirement

    * push coverage only for requirements-extras test

    * gpfa verbose flag; added licence in the docs

    * Fixed and extended documentation, removed misleading function outputs.

commit d6822d6
Author: Danylo Ulianych <dizcza@gmail.com>
Date:   Tue Jul 23 16:35:45 2019 +0300

    Acknowledgments (NeuralEnsemble#241)

    * acknowledgments

    * removed reference file AUTHORS.txt

    * recursive-exclude . *~

commit 6fd3a5b
Author: Danylo Ulianych <dizcza@gmail.com>
Date:   Mon Jul 22 23:14:36 2019 +0300

    Release v0.6.3 (NeuralEnsemble#240)

    * Release v0.6.3

    * Update doc/reference/waveform_features.rst

    Co-Authored-By: Michael Denker <m.denker@fz-juelich.de>

commit c013e39
Author: Danylo Ulianych <dizcza@gmail.com>
Date:   Sun Jul 21 09:18:03 2019 +0300

    elephant dir packages are back (NeuralEnsemble#239)

    * reverted removed imports of elephant's internal packages; download fim during the setup

    * download fim from tools/fim_manager.py

    * dummy tools/__init__.py to support py2

    * include requirements in MANIFEST back

    * fixed spade licence typo

    * included waveform_features module; removed tools; don't donwload fim at setup while macking a tarball with 'sdist' command

    travis not fixed downloading fim

    * skip time consuming test_spade_cpp if not HAVE_FIM

    * travis pip install generated tarball

    * recursive-include elephant *.py

commit 3c30574
Author: Danylo Ulianych <dizcza@gmail.com>
Date:   Wed Jul 10 18:21:37 2019 +0200

    Butterworth supports sosfiltfilt filter_function (NeuralEnsemble#234)

    * Butterworth supports sosfiltfilt filter_function

    * higher order filters comment
@Moritz-Alexander-Kern Moritz-Alexander-Kern deleted the elephant_dir_packages branch September 6, 2022 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] import elephant rendered useless by removal of imports in __init__.py
3 participants