Skip to content
This repository has been archived by the owner on Sep 1, 2023. It is now read-only.

Put all functionally of "CMakeLists.txt" into "setup.py" #1573

Closed
david-ragazzi opened this issue Nov 26, 2014 · 16 comments · Fixed by #1579 or #1771
Closed

Put all functionally of "CMakeLists.txt" into "setup.py" #1573

david-ragazzi opened this issue Nov 26, 2014 · 16 comments · Fixed by #1579 or #1771
Assignees
Milestone

Comments

@david-ragazzi
Copy link
Contributor

Once that theoretically all extensions could be build using SetupTools (
https://docs.python.org/2/extending/building.html) and that nupic.cpp (nupic.core) now can be build separately, we could remove CMake from NuPIC Pyhton and leave it only in NuPIC C++.

This will decrease the dependencies in addition to unify the process* into setup.py.

*Again: we DO need be very careful on avoid creating several files to perform build operations instead of centralize on setup.py, I'm saying this due to our bad experience with the old build system.

@david-ragazzi
Copy link
Contributor Author

@rhyolight Assign it to me, please.

@rhyolight rhyolight added this to the 0.1.0 milestone Nov 27, 2014
@rhyolight rhyolight changed the title Put all functionatly of "CMakeLists.txt" into "setup.py" Put all functionally of "CMakeLists.txt" into "setup.py" Nov 27, 2014
@david-ragazzi
Copy link
Contributor Author

Hopefully this work will help on the "pip installable" problem.. I'm working hard on this and in the next week I'll create a PR to you follow my progress..

@rhyolight
Copy link
Member

Great! Thank you!

@breznak
Copy link
Member

breznak commented Dec 31, 2014

I have a suggestion to even more simplify the build process:

  • user: setup.py downloads the published binary tarball
  • developer:
    ** compiles nupic.core himself
    ** points setup.py to the binary release

So we'd drop the option of "compiles from source without user intervention", which is kind of pointless, I think.

This will fix the problem where changes in nupic.core CMakeLists have to be propagated to nupic CMakeLists, resp. nupic setup.py

What do you think @david-ragazzi @rhyolight ?

@david-ragazzi
Copy link
Contributor Author

user: setup.py downloads the published binary tarball

@breznak What you meant by published binary tarball? The bindings also would be included in the tarbal?

In either case, I think it's better merge #1579 first as it is done..

@breznak
Copy link
Member

breznak commented Dec 31, 2014

@david-ragazzi

What you meant by published binary tarball? The bindings also would be included in the tarbal?

"published binary tarball" - the nupic.core release that is now automatically published to the amazon server.
the simplification would be setup.py would not have to be able to build nupic.core form source - either it's downloaded from the published binary, or up to the developer to compile it and point setup.py to the release directory.

@rhyolight
Copy link
Member

The only reason I kept the "compiles from local source" option was to allow nupic to build without internet access if nupic.core was checked out locally. But the truth is if the user already had the nupic.core binary tarball cached locally, they would not need access to re-build.

So I think it's okay to remove the "compiles from local source" option, but interested in what @oxtopus thinks.

@oxtopus
Copy link
Contributor

oxtopus commented Jan 2, 2015

Agreed. No need for "compiles from local source", the user either downloads it automatically as part of setup, or the developer builds it separately.

@david-ragazzi
Copy link
Contributor Author

"published binary tarball" - the nupic.core release that is now automatically published to the amazon server.
the simplification would be setup.py would not have to be able to build nupic.core form source - either it's downloaded from the published binary, or up to the developer to compile it and point setup.py to the release directory.

I agree with this. This would avoid a lot of install issues: c++ compiler problems, problematic environment, etc. We could have an option (--build-locally?) to be passed to setup.py that turn on the build from local source. If no option is passed, it simply would download the tarball by default. This would make the installation quicker and simpler. However, how about we create an issue for this? Otherwise, #1579 (current install process) never will be merged.. 😞

@breznak
Copy link
Member

breznak commented Jan 5, 2015

👍 no problem with next iteration step, after this PR is merged. But the
point was to decide, and if we dont want it, cut the code.

@david-ragazzi
Copy link
Contributor Author

no problem with next iteration step, after this PR is merged.

Ok, could you create an issue for that?

@rhyolight
Copy link
Member

Ok, could you create an issue for that?

#1713

@david-ragazzi
Copy link
Contributor Author

@rhyolight I have seen that you removed nupic.core local build as mentioned by @breznak . Thanks for this!

So once this is merged we could remove CMake and Make from "Dependencies" section at README:

Dependencies:

  • Python 2.7 (with development headers)
  • Compiler toolchain with support for C++11 such as GCC >= 4.7 (Linux-only), or llvm/clang
  • Make
  • CMake

If in the future, we publish bindings binaries to S3, we also could remove GCC/Clang mentions.

@breznak
Copy link
Member

breznak commented Jan 6, 2015

So once this is merged we could remove CMake and Make from "Dependencies" section at README:

Then we should put it in the "developer's build" page

@david-ragazzi
Copy link
Contributor Author

So once this is merged we could remove CMake and Make from "Dependencies" section at README:

Then we should put it in the "developer's build" page

@breznak I didn't understand. Once developers will build nupic.core separately, it's supposed that they will have cloned nupic.core repo and read its README (which has dependencies mentioned) to build it. From I understood, now setup.py only points to binaries build by Travis or the user, it doesn't build nupic.core longer.

@breznak
Copy link
Member

breznak commented Jan 6, 2015

nupic.core repo and read its README

@david-ragazzi my bad, this is true!

david-ragazzi pushed a commit to david-ragazzi/nupic that referenced this issue Jan 22, 2015
Fixes: numenta#1573

It's weird.. I simply don't undertand why this file still is in repository. I thought I had removed it through numenta#1579
mihail911 pushed a commit to mihail911/nupic that referenced this issue Aug 5, 2015
Fixes: numenta#1573

It's weird.. I simply don't undertand why this file still is in repository. I thought I had removed it through numenta#1579
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.