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

Support editable install #93

Merged
merged 5 commits into from
Feb 2, 2022
Merged

Support editable install #93

merged 5 commits into from
Feb 2, 2022

Conversation

matham
Copy link
Contributor

@matham matham commented Nov 22, 2021

Currently, to use lava from a cloned repo, you have to mess with the PYTHONPATH which is not ideal.

Typically, this is avoided by supporting an editable install. Then, you just have to do pip install -e . from the root, and things will work as any normally installed package. The problem is that you need to specify setuptools as a dependency in pyproject.toml.

  • Additionally, I updated to docs to show how to use the editable install, rather than messing with the PYTHONPATH.
  • I also updated the instructions for Windows, where python is python, not python3.
  • Also pip install -U pip didn't work failing with some kind of permission error if I can remember correctly. So python -m pip install --upgrade pip seemed like the more normal and less likely to fail command. I didn't update the linux instructions for this, although you probably should, since I didn't test.
  • In windows, activate.bat is actually under Scripts, not bin. So I updated that.
  • Unfortunately, the command is still wrong, because if you use a bash shell on On Windows, like me, the command is source python3_venv/Scripts/activate without the .bat suffix. If you use the normal cmd/powershell terminal, then source doesn't work so you just do python3_venv/Scripts/activate.bat. But, I couldn't test this right now on cmd, so I didn't change this part.

On my system, with the changes it worked:

$ pyb -E unit
PyBuilder version 0.13.3
Build started at 2021-11-22 17:53:27
------------------------------------------------------------
[INFO]  Installing or updating plugin "pypi:pybuilder_bandit, module name 'pybuilder_bandit'"
[INFO]  Processing plugin packages 'pybuilder_bandit' to be installed with {}
[INFO]  Activated environments: unit
[INFO]  Building lava-nc version 0.1.0
[INFO]  Executing build in g:\python\lava
[INFO]  Going to execute tasks: analyze, publish
[INFO]  Processing plugin packages 'coverage~=5.2' to be installed with {'upgrade': True}
[INFO]  Processing plugin packages 'flake8~=3.7' to be installed with {'upgrade': True}
[INFO]  Processing plugin packages 'pypandoc~=1.4' to be installed with {'upgrade': True}
[INFO]  Processing plugin packages 'setuptools>=38.6.0' to be installed with {'upgrade': True}
[INFO]  Processing plugin packages 'sphinx_rtd_theme' to be installed with {}
[INFO]  Processing plugin packages 'sphinx_tabs' to be installed with {}
[INFO]  Processing plugin packages 'twine>=1.15.0' to be installed with {'upgrade': True}
[INFO]  Processing plugin packages 'unittest-xml-reporting~=3.0.4' to be installed with {'upgrade': True}
[INFO]  Processing plugin packages 'wheel>=0.34.0' to be installed with {'upgrade': True}
[INFO]  Creating target 'build' VEnv in 'g:\python\lava\target\venv\build\cpython-3.10.0.final.0'
[INFO]  Processing dependency packages 'requirements.txt' to be installed with {}
[INFO]  Creating target 'test' VEnv in 'g:\python\lava\target\venv\test\cpython-3.10.0.final.0'
[INFO]  Processing dependency packages 'requirements.txt' to be installed with {}
[INFO]  Requested coverage for tasks: pybuilder.plugins.python.unittest_plugin:run_unit_tests
[INFO]  Running unit tests
[INFO]  Executing unit tests from Python modules in g:\python\lava\tests
Runtime not started yet.
g:\python\lava\src\lava\proc\lif\models.py:129: RuntimeWarning: divide by zero encountered in remainder
  wrapped_curr = np.mod(decayed_curr,
[INFO]  Executed 108 unit tests
[INFO]  All unit tests passed.
[INFO]  Executing flake8 on project sources.
[INFO]  Building distribution in g:\python\lava\target\dist\lava-nc-0.1.0
[INFO]  Copying scripts to g:\python\lava\target\dist\lava-nc-0.1.0\scripts
[INFO]  Writing setup.py as g:\python\lava\target\dist\lava-nc-0.1.0\setup.py
[INFO]  Collecting coverage information for 'pybuilder.plugins.python.unittest_plugin:run_unit_tests'
[WARN]  ut_coverage_branch_threshold_warn is 0 and branch coverage will not be checked
[WARN]  ut_coverage_branch_partial_threshold_warn is 0 and partial branch coverage will not be checked
[INFO]  Running unit tests
[INFO]  Executing unit tests from Python modules in g:\python\lava\tests
Runtime not started yet.
g:\python\lava\src\lava\proc\lif\models.py:129: RuntimeWarning: divide by zero encountered in remainder
  wrapped_curr = np.mod(decayed_curr,
[INFO]  Executed 108 unit tests
[INFO]  All unit tests passed.
[WARN]  Test coverage below 70% for lava:  0%
[WARN]  Test coverage below 70% for lava.magma:  0%
[WARN]  Test coverage below 70% for lava.magma.compiler:  0%
[WARN]  Test coverage below 70% for lava.magma.compiler.c:  0%
[WARN]  Test coverage below 70% for lava.magma.compiler.channels:  0%
[WARN]  Test coverage below 70% for lava.magma.compiler.nc:  0%
[WARN]  Test coverage below 70% for lava.magma.compiler.py:  0%
[WARN]  Test coverage below 70% for lava.magma.core.run_configs: 50%
[WARN]  Test coverage below 70% for lava.magma.core:  0%
[WARN]  Test coverage below 70% for lava.magma.core.model.model: 65%
[WARN]  Test coverage below 70% for lava.magma.core.model.c.type:  0%
[WARN]  Test coverage below 70% for lava.magma.core.model.nc.model: 43%
[WARN]  Test coverage below 70% for lava.magma.core.model.py.model: 39%
[WARN]  Test coverage below 70% for lava.magma.core.model.py.ports: 68%
[WARN]  Test coverage below 70% for lava.magma.runtime.runtime_service: 31%
[WARN]  Test coverage below 70% for lava.magma.runtime:  0%
[WARN]  Test coverage below 70% for lava.magma.runtime.channels:  0%
[WARN]  Test coverage below 70% for lava.magma.runtime.node:  0%
[WARN]  Test coverage below 70% for lava.proc.io:  0%
[WARN]  Test coverage below 70% for lava.proc.lif.models: 45%
[WARN]  Test coverage below 70% for lava.proc.sparse:  0%
[WARN]  Test coverage below 70% for lava.tutorials:  0%
[WARN]  Test coverage below 70% for lava.utils.float2fixed:  0%
[WARN]  Test coverage below 70% for lava.utils.profiler:  0%
[WARN]  Test coverage below 70% for lava.utils.validator:  0%
[WARN]  Test coverage below 70% for lava.utils.visualizer:  0%
[WARN]  Test coverage below 70% for lava.utils:  0%
[WARN]  Test coverage below 70% for lava.utils.dataloader.mnist:  0%
[WARN]  Test coverage below 70% for lava.utils.dataloader:  0%
[INFO]  Overall pybuilder.plugins.python.unittest_plugin.run_unit_tests coverage is 75%
[INFO]  Overall pybuilder.plugins.python.unittest_plugin.run_unit_tests branch coverage is 59%
[INFO]  Overall pybuilder.plugins.python.unittest_plugin.run_unit_tests partial branch coverage is 89%
[WARN]  Test coverage below 70% for lava:  0%
[WARN]  Test coverage below 70% for lava.magma:  0%
[WARN]  Test coverage below 70% for lava.magma.compiler:  0%
[WARN]  Test coverage below 70% for lava.magma.compiler.c:  0%
[WARN]  Test coverage below 70% for lava.magma.compiler.channels:  0%
[WARN]  Test coverage below 70% for lava.magma.compiler.nc:  0%
[WARN]  Test coverage below 70% for lava.magma.compiler.py:  0%
[WARN]  Test coverage below 70% for lava.magma.core.run_configs: 50%
[WARN]  Test coverage below 70% for lava.magma.core:  0%
[WARN]  Test coverage below 70% for lava.magma.core.model.model: 65%
[WARN]  Test coverage below 70% for lava.magma.core.model.c.type:  0%
[WARN]  Test coverage below 70% for lava.magma.core.model.nc.model: 43%
[WARN]  Test coverage below 70% for lava.magma.core.model.py.model: 39%
[WARN]  Test coverage below 70% for lava.magma.core.model.py.ports: 68%
[WARN]  Test coverage below 70% for lava.magma.runtime.runtime_service: 31%
[WARN]  Test coverage below 70% for lava.magma.runtime:  0%
[WARN]  Test coverage below 70% for lava.magma.runtime.channels:  0%
[WARN]  Test coverage below 70% for lava.magma.runtime.node:  0%
[WARN]  Test coverage below 70% for lava.proc.io:  0%
[WARN]  Test coverage below 70% for lava.proc.lif.models: 45%
[WARN]  Test coverage below 70% for lava.proc.sparse:  0%
[WARN]  Test coverage below 70% for lava.tutorials:  0%
[WARN]  Test coverage below 70% for lava.utils.float2fixed:  0%
[WARN]  Test coverage below 70% for lava.utils.profiler:  0%
[WARN]  Test coverage below 70% for lava.utils.validator:  0%
[WARN]  Test coverage below 70% for lava.utils.visualizer:  0%
[WARN]  Test coverage below 70% for lava.utils:  0%
[WARN]  Test coverage below 70% for lava.utils.dataloader.mnist:  0%
[WARN]  Test coverage below 70% for lava.utils.dataloader:  0%
[INFO]  Overall lava-nc coverage is 75%
[INFO]  Overall lava-nc branch coverage is 59%
[INFO]  Overall lava-nc partial branch coverage is 89%
[INFO]  Building binary distribution in g:\python\lava\target\dist\lava-nc-0.1.0
[INFO]  Running Twine check for generated artifacts
------------------------------------------------------------
BUILD SUCCESSFUL
------------------------------------------------------------
Build Summary
             Project: lava-nc
             Version: 0.1.0
      Base directory: g:\python\lava
        Environments: unit
               Tasks: prepare [17277 ms] compile_sources [0 ms] run_unit_tests [19036 ms] analyze [2712 ms] package [88 ms] run_integration_tests [0 ms] verify [0 ms] coverage [24489 ms] publish [5942 ms]
Build finished at 2021-11-22 17:54:42
Build took 75 seconds (75565 ms)

@mgkwill mgkwill self-requested a review November 22, 2021 23:29
@mgkwill mgkwill linked an issue Nov 22, 2021 that may be closed by this pull request
@mgkwill
Copy link
Contributor

mgkwill commented Nov 22, 2021

Hi @matham thanks for this PR!

A portion of this PR looks like it solves current issue #19. Thanks!

I'll review the editable install portion and try it out locally and then get back to you.

Generally I've been able to install from lava repo with pip install . using our current setup. As far as the PYTHONPATH instructions there was some need to provide all possible install avenues including exotic developer setups.

@matham
Copy link
Contributor Author

matham commented Nov 22, 2021

As far as the PYTHONPATH instructions there was some need to provide all possible install avenues including exotic developer setups.

Makes sense. Feel free to edit the PR code directly. Or I can do it as well if you specific changes in mind.

@awintel
Copy link
Contributor

awintel commented Nov 23, 2021

It probably makes sense to offer pip install -e . as an option in our instructions.

@mgkwill mgkwill added this to the install_issues milestone Dec 2, 2021
Copy link
Contributor

@mgkwill mgkwill left a comment

Choose a reason for hiding this comment

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

+1.

Install works locally and should ease development install.

@mgkwill
Copy link
Contributor

mgkwill commented Feb 2, 2022

Sorry about the long review wait @matham. This is mostly down to holidays and some near term deadlines.

@mgkwill
Copy link
Contributor

mgkwill commented Feb 2, 2022

Once this is merged we need to update the FAQ with updated install instructions https://github.com/lava-nc/lava/wiki/Frequently-Asked-Questions-(FAQ) and official docs need to be updated at least here
https://github.com/lava-nc/lava-docs/blame/main/developer_guide.rst#L161-L186
possibly in some other places.

Copy link
Contributor

@PhilippPlank PhilippPlank left a comment

Choose a reason for hiding this comment

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

Looks good. Does this solve Issue #19?

@mgkwill
Copy link
Contributor

mgkwill commented Feb 2, 2022

Looks good. Does this solve Issue #19?

Thanks for the review @PhilippPlank! Yes it solves #19, as I mentioned in my first comment. I've also linked it to #19.

@mgkwill mgkwill merged commit 85c36b5 into lava-nc:main Feb 2, 2022
@matham matham deleted the patch-1 branch February 2, 2022 22:03
monkin77 pushed a commit to monkin77/thesis-lava that referenced this pull request Jul 12, 2024
* Add setuptools to support editable install.

* Update install docs to support editable install.
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.

source command in Windows install instructions does not exist
4 participants