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

Remove dependency on NUPIC_CORE_RELEASE environment variable #552

Merged
merged 6 commits into from Aug 25, 2015

Conversation

akhilaananthram
Copy link
Contributor

Now the user must use the flag --nupic-core-dir when calling setup.py. This should allow us to eliminate the setup.py that is at the top level as well since that was just there to run "pip wheel ." and we no longer run that.

Fixes #551

@oxtopus @scottpurdy

@akhilaananthram
Copy link
Contributor Author

Actually hold off. I just realized that this doesn't include the egg anymore and that I need to figure out a way to include that

@akhilaananthram
Copy link
Contributor Author

@oxtopus @scottpurdy: I added the egg as well

pip wheel --wheel-dir=dist/wheels .
echo "pip wheel -r requirements.txt"
pip wheel --wheel-dir=dist/wheels -r requirements.txt
echo "python setup.py bdist_wheel"
Copy link
Contributor

Choose a reason for hiding this comment

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

Better to set -o xtrace rather than include echos in the shell scripts.

# Build all NuPIC and all required python packages into dist/wheels as .whl
# files.
echo "pip wheel --wheel-dir=dist/wheels ."
pip wheel --wheel-dir=dist/wheels .
pip wheel --wheel-dir=dist/wheels -r requirements.txt
Copy link
Contributor

Choose a reason for hiding this comment

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

won't this choke on nupic.bindings in requirements.txt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is in nupic.core. nupic.bindings isn't in requirements.txt because these are the requirements for nupic.bindings

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@scottpurdy
Copy link
Contributor

@oxtopus - please merge when you're happy with this

@akhilaananthram
Copy link
Contributor Author

Let's wait on this. I want to test it a bit more

@akhilaananthram
Copy link
Contributor Author

@oxtopus: I think this is ready. Please let me know what you think

@rhyolight
Copy link
Member

This doesn't actually remove the dependency? An exception is still raised if the var is missing. Am I missing something?

@akhilaananthram
Copy link
Contributor Author

The exception is raised if the --nupic-core-dir is not passed in. Setup.py is not longer dependent on the user setting the environment variable NUPIC_CORE_RELEASE, but rather its on an input flag, which matches the format of nupic's setup.py. I think this is just so the user doesn't have to run the additional command to set the environment variable and because this is a more standard format.

@scottpurdy
Copy link
Contributor

@akhilaananthram - If we do a PyPI release of nupic.bindings, will it be able to find the included cmake stuff without needing this flag?

@akhilaananthram
Copy link
Contributor Author

Not for source builds. You would need to provide the flag. The binaries should be fine though.

@rhyolight
Copy link
Member

@scottpurdy What is the status of this? LGTM.

scottpurdy added a commit that referenced this pull request Aug 25, 2015
Remove dependency on NUPIC_CORE_RELEASE environment variable
@scottpurdy scottpurdy merged commit 745a147 into numenta:master Aug 25, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants