search

isce-framework / fringe

Fine Resolution InSAR With Generalized Eigenvectors (FRInGE)
Apache License 2.0
80 stars 42 forks source link

update paths when using Ananaconda #20

Closed jlmaurer closed 3 years ago

jlmaurer commented 4 years ago
rtburns-jpl commented 4 years ago

Alternatively you can invoke cython with python -m cython, that's what I do in the cmake files for isce2/3. That way it's guaranteed to work with the python version you're using https://github.com/isce-framework/isce2/blob/master/.cmake/FindCython.cmake

rtburns-jpl commented 4 years ago

Also, when you're using anaconda, I recommend passing -DCMAKE_PREFIX_PATH=$CONDA_PREFIX to cmake. This has the added benefit of finding other packages as well, and you don't have to remember the name of the active env

jlmaurer commented 4 years ago

@rtburns-jpl and @piyushrpt, do you recommend I add the '''-DCMAKE_PREFIX_PATH=$CONDA_PREFIX''' argument to the current PR? This would I think also require adding an export command for the CONDA_PREFIX.

rtburns-jpl commented 4 years ago

You'd provide -DCMAKE_PREFIX_PATH=$CONDA_PREFIX as an arg to cmake, i.e. alongside -DCMAKE_INSTALL_PREFIX=... - no need to export. I think this is generally preferable to setting *_ROOT variables.

rtburns-jpl commented 4 years ago

Oh, I see what you mean about the CONDA_PREFIX export - that variable is automatically set by conda to your currently active environment, so no need to export it yourself :)

jlmaurer commented 4 years ago

@rtburns-jpl Got it. I'll add the command arg to the install.md example.

piyushrpt commented 4 years ago

When using conda compilers, you dont need to specify CXX 

CXX=${CXX}

is redundant

hfattahi commented 3 years ago

Hi @jlmaurer, The https://github.com/isce-framework/fringe/pull/51 fixes the build on ci and also on local linux machine with conda. As mentioned here and in https://github.com/isce-framework/fringe/pull/51 we don't want to change the order for cython. My suggestion is to update this PR by removing the change to CMake for finding cython and with special cases where we specifically need to enforce such as the following: 

-DCYTHON_EXECUTABLE=$CONDA_PREFIX/bin/cython

Or we can just close this PR and update the installation in a separate PR. Let me know what makes better for you.

hfattahi commented 3 years ago

@jlmaurer I'll close this PR for now as I'm going to rename the master to main. Feel free to re-open and update please if you think it is still needed.