Skip to content

[WIP] PythonHome Path is correctly set #279

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

Closed
wants to merge 2 commits into from
Closed

[WIP] PythonHome Path is correctly set #279

wants to merge 2 commits into from

Conversation

den-run-ai
Copy link
Contributor

@den-run-ai den-run-ai commented Oct 30, 2016

Blank Description


Pending for merge:

  • Generalize/Improve Unittests (has to be able to run in Appveyor, Local (travis could wait since embdded tests arent working there)

  • Rebase and fix merge conflicts. (@vmuriart)

@den-run-ai den-run-ai changed the title {WIP] PythonHome Path is correctly set [WIP] PythonHome Path is correctly set Oct 30, 2016
@filmor
Copy link
Member

filmor commented Nov 23, 2016

What problem does this fix? Is it still valid?

@den-run-ai
Copy link
Contributor Author

Here is the original pull request, I think it is valid fix and the leak is unavoidable and non-essential.

#186

@vmuriart
Copy link
Contributor

vmuriart commented Feb 2, 2017

@filmor this is probably the issue they were trying to solve

http://stackoverflow.com/questions/5694706/py-initialize-fails-unable-to-load-the-file-system-codec

I'm having that issue right now trying to run python3 locally...

@den-run-ai
Copy link
Contributor Author

Exactly!

@vmuriart
Copy link
Contributor

vmuriart commented Feb 2, 2017

@denfromufa are you/have you had that issue before? I normally pawn off all my Python3 testing to appveyor/travis so never really encountered this before

@den-run-ai
Copy link
Contributor Author

den-run-ai commented Feb 2, 2017 via email

@vmuriart
Copy link
Contributor

vmuriart commented Feb 7, 2017

@denfromufa @filmor Anything missing or needed to close/merge this pr? I'll do the rebase and fix the merge conflicts if we are good to merge this in.

We should probably update the other open pull requests as well with current status/pending items.
I can similarly take care of the rebase/merge once they are ready to merge.

@den-run-ai
Copy link
Contributor Author

den-run-ai commented Feb 7, 2017

The unit test definitely needs to be improved

@vmuriart
Copy link
Contributor

vmuriart commented Feb 7, 2017

yup I can see that now. Going to be tricky figuring out how to generalize that one so that it works on Appveyor (could use enviroment variables) & Local.

hm... that reminds me, where in the code do we search for the python dll's? I couldn't find it when I posted that SO question a few comments back. I couldn't get Python3 to run embedded test no matter way environment variable i update.

@den-run-ai
Copy link
Contributor Author

@vmuriart you mean that Python 3 embedded tests are not working when you have multiple Python runtimes installed on your Windows machine?

@den-run-ai
Copy link
Contributor Author

Is nPython working?

@vmuriart
Copy link
Contributor

vmuriart commented Feb 7, 2017

That was actually the first time I used nPythonand it didn't work either. The embedded tests weren't working on python36 when python27 was my default python (and original) python install. I updated all references to python27 in my enviroment varialbles and added new ones but nothing worked.

I got it working finally by uninstalling Anaconda Python2.7 and deleting all my python installations. I reinstalled Anaconda Python, but 3.6 this time around as the default. I reinstalled all my python interpreters (including 27) and everything is working now.

I can test on Python 3.6 by default with no issues, and I can test Python 2.7 after updating PATH. I have no idea why it didn't work the other way around though.

@vmuriart vmuriart mentioned this pull request Mar 3, 2017
2 tasks
@den-run-ai den-run-ai closed this Mar 3, 2017
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.

4 participants