Skip to content

[WIP] PythonHome Path is correctly set#279

Closed
den-run-ai wants to merge 2 commits into
pythonnet:masterfrom
leomeuk:develop
Closed

[WIP] PythonHome Path is correctly set#279
den-run-ai wants to merge 2 commits into
pythonnet:masterfrom
leomeuk:develop

Conversation

@den-run-ai

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

Copy link
Copy Markdown
Contributor

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

filmor commented Nov 23, 2016

Copy link
Copy Markdown
Member

What problem does this fix? Is it still valid?

@den-run-ai

Copy link
Copy Markdown
Contributor Author

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

#186

@vmuriart

vmuriart commented Feb 2, 2017

Copy link
Copy Markdown
Contributor

@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
Copy Markdown
Contributor Author

Exactly!

@vmuriart

vmuriart commented Feb 2, 2017

Copy link
Copy Markdown
Contributor

@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

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

Copy link
Copy Markdown
Contributor Author

@vmuriart

vmuriart commented Feb 7, 2017

Copy link
Copy Markdown
Contributor

@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

den-run-ai commented Feb 7, 2017

Copy link
Copy Markdown
Contributor Author

The unit test definitely needs to be improved

@vmuriart

vmuriart commented Feb 7, 2017

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
Contributor Author

Is nPython working?

@vmuriart

vmuriart commented Feb 7, 2017

Copy link
Copy Markdown
Contributor

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