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

Remove pyinstall #9

Open
adler-j opened this issue Nov 27, 2015 · 12 comments
Open

Remove pyinstall #9

adler-j opened this issue Nov 27, 2015 · 12 comments
Assignees
Labels

Comments

@adler-j
Copy link
Member

adler-j commented Nov 27, 2015

We should change remove the custom pyinstall and instead use the built in install found in CMake. This is used in our STIR clone.

@kohr-h
Copy link
Member

kohr-h commented Dec 8, 2015

And use a config flag --use-python? Sounds reasonable.

@adler-j
Copy link
Member Author

adler-j commented Dec 8, 2015

I've done some "research" on this and it seems harder than youd expected. We'll see if it happens.

Regarding a --use-python flag I don't see the point, the library is exclusively meant to build a python library. I recently removed the USE_PYTHON CMake flag since it didn't really make sense.

@kohr-h
Copy link
Member

kohr-h commented Dec 8, 2015

You mean it is problematic to change the behavior of make install to do what make pyinstall does now?

What about making the "post-build-and-pre-install" actions part of the build (pybuild maybe) and do the actual installation in the setup.py? Or put all the pyinstall stuff into the setup.py install?

@adler-j
Copy link
Member Author

adler-j commented Dec 8, 2015

I think the big move would be to move the whole build system to setup.py instead of MakeFiles directly, I'm not much of an expert there though and it is not priority for now.

@adler-j
Copy link
Member Author

adler-j commented Dec 8, 2015

We first need to get the core library to PyPI before it becomes a more urgent issue.

@kohr-h
Copy link
Member

kohr-h commented Dec 8, 2015

I see. Well, maybe I can for now just "hack in" the system installation of odlpp by generating symbolic links in the virtualenv. Then we can run CUDA tests already today. I'll try it.

@adler-j
Copy link
Member Author

adler-j commented Dec 8, 2015

That is more related to #10, with that fixed you could select which python to call which should solve your issue?

@kohr-h
Copy link
Member

kohr-h commented Dec 8, 2015

In principle, yes. I need to check if it is possible to install in Python 3. Last time it failed, but that was ages ago.

@adler-j
Copy link
Member Author

adler-j commented Dec 8, 2015

The code currently runs python in a terminal, changing it should change where it is installed etc.

@adler-j
Copy link
Member Author

adler-j commented Dec 8, 2015

It seems moving to a full setup.py style installation is complicated, matplotlib has a total of about 2500 lines in their build scripts. Basically you need to wrap everything you need yourself it seems. I'm putting it of for now.

Allowing users to select what python version to install with is however preferable and I'll see what can be done.

@kohr-h
Copy link
Member

kohr-h commented Dec 8, 2015

The CMake recipe has a TARGET_PY variable already, is that the python interpreter to be used or is it the "python build target"?

@adler-j
Copy link
Member Author

adler-j commented Dec 8, 2015

TARGET_PY is slightly badly named, it is simply where the binaries were installed and where the __init__.py file should go so setup.py can be called on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants