yotarazona / scikit-eo

A Python package for Remote Sensing Data Analysis
https://yotarazona.github.io/scikit-eo/
Other
15 stars 2 forks source link

Tensorflow import error in notebook 11 #10

Closed dbuscombe-usgs closed 3 weeks ago

dbuscombe-usgs commented 1 month ago

ping https://github.com/openjournals/joss-reviews/issues/6692

Tensorflow is not a listed dependency for your package. See https://github.com/yotarazona/scikit-eo/issues/6. This means that notebook 11 fails

I have tensorflow installed, and can verify using python and ipython terminals, as well as jupyter. I can import import tensorflow as tf with no error

However, when I attempt from scikeo.deeplearning import DL, I get the following error ModuleNotFoundError: No module named 'tensorflow.keras'

It appears the packaging issues I've already identified are at play here (i.e., these are all parts of the same problem). In this case, it seems to me that the from scikeo.deeplearning module is improperly linked against tensorflow, or perhaps the conda environment I had to create to run these codes was not done so optimally? For example, I regularly have to install tensorflow in some specific order.

The packages I maintained usually spend quite a significant amount of time ensuring that all the dependencies are installable together in a stable way, usually tested on multiple machines. I recommend that here, coming up with a conda or virtual environment recipe that is known to work for all the provided functionality of this software.

dbuscombe-usgs commented 1 month ago

I was able to fix this here https://github.com/yotarazona/scikit-eo/issues/5#issuecomment-2136179777

I would suggest this way of importing keras to avoid those import errors

yotarazona commented 3 weeks ago

Hi @dbuscombe-usgs, thanks a lot for open this issue!. After some trial and error the package is running well now. tensorflow, rasterio and other required dependencies will be installed when scikit-eo is installed. Additionally, scikit-eo was tested on multiple machines in order to make sure everything is working out. Please see issue number #5.

KBodolai commented 2 weeks ago

Hi @yotarazona , I'm moving on with my side of the review, and I have a question about this.

Right now it installs everything, which is the most straightforward way, however, tensorflow makes the installation quite heavy.

I've seen in the past packages that have the possibility of using neural networks but where it's not a strict requirement treat it as an optional dependency, that can be installed if you do something like pip install scikeo[neural-nets].

I think it could improve the user experience, as long as it's documented in the installation instructions, although it depends a lot on what you expect the average user to be doing!