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

Dev install missing openeo-udf? #159

Closed
m-mohr opened this issue Oct 23, 2020 · 8 comments
Closed

Dev install missing openeo-udf? #159

m-mohr opened this issue Oct 23, 2020 · 8 comments
Labels

Comments

@m-mohr
Copy link
Member

m-mohr commented Oct 23, 2020

I'm trying to install the client for development purposes (want to try the _repl_html_ in Jupyter), but after executing pip install -e .[dev] I receive that openeo-udf is missing:

PS C:\Dev\openeo-python-client> pip install -e .[dev]
Obtaining file:///C:/Dev/openeo-python-client
Requirement already satisfied: requests in c:\programdata\anaconda3\lib\site-packages (from openeo==0.4.8a1) (2.22.0)
Requirement already satisfied: requests_mock in c:\programdata\anaconda3\lib\site-packages (from openeo==0.4.8a1) (1.5.0)
Requirement already satisfied: shapely>=1.6.4 in c:\programdata\anaconda3\lib\site-packages (from openeo==0.4.8a1) (1.6.4)
Collecting numpy>=1.17.0
  Downloading numpy-1.19.2-cp36-cp36m-win_amd64.whl (12.9 MB)
     |████████████████████████████████| 12.9 MB 3.2 MB/s
Collecting deprecated
  Downloading Deprecated-1.2.10-py2.py3-none-any.whl (8.7 kB)
Requirement already satisfied: pandas>0.20.0 in c:\programdata\anaconda3\lib\site-packages (from openeo==0.4.8a1) (0.23.0)
Requirement already satisfied: pytest>=4.5.0 in c:\programdata\anaconda3\lib\site-packages (from openeo==0.4.8a1) (4.6.2)
Requirement already satisfied: mock in c:\programdata\anaconda3\lib\site-packages (from openeo==0.4.8a1) (2.0.0)
Collecting xarray==0.12.3
  Downloading xarray-0.12.3-py2.py3-none-any.whl (566 kB)
     |████████████████████████████████| 566 kB ...
Collecting h5netcdf
  Downloading h5netcdf-0.8.1-py2.py3-none-any.whl (18 kB)
ERROR: Could not find a version that satisfies the requirement openeo-udf>=1.0.0rc2 (from openeo==0.4.8a1) (from versions: none)
ERROR: No matching distribution found for openeo-udf>=1.0.0rc2 (from openeo==0.4.8a1)
PS C:\Dev\openeo-python-client>
@m-mohr
Copy link
Member Author

m-mohr commented Oct 23, 2020

Removing the missing dependency from setup.py helps for now...

@jdries
Copy link
Collaborator

jdries commented Oct 23, 2020

yes, other workaround is to also clone openeo_udf and do pip install -e .
or not use [dev]
or install it from this wheel: https://artifactory.vgt.vito.be/webapp/#/artifacts/browse/tree/General/python-openeo/openeo_udf/1.0.0rc2/openeo_udf-1.0.0rc2-py2.py3-none-any.whl

It's on our list to do a proper release on pypi. But in fact we would like to find a proper way to depend on openeo-udf without dragging in the whole server implementation.

@soxofaan
Copy link
Member

But in fact we would like to find a proper way to depend on openeo-udf without dragging in the whole server implementation.

+1 for me

reference: internal ticket EP-3578

Currently, openeo-python-client has an optional dependency on openeo-udf (through local imports in the client implementation). The openeo-python-client tests have a hard dependency (global imports).

The dependency on openeo-udf is however not ideal, as this drags in a lot of secondary dependencies that are overkill for general openeo-python-client usage.

possible solutions:

  • move openeo-udf's DataCube (or at least a base class of it) to openeo-python-client
  • make it possible to depend on a slimmed down version of openeo-udf
  • ...?

@m-mohr
Copy link
Member Author

m-mohr commented Oct 27, 2020

I think openeo-udf should only contain the underlying specification of the protocol and base classes, while openeo-python-udf should be the actual python server implementation (like openeo-r-udf). openeo-python-udf can then inherit from openeo-udf, but I'm not sure how easy all that would be as I think Sören implemented things in a way that documentation is generated from code.

@soxofaan
Copy link
Member

openeo-udf should only contain the underlying specification of the protocol and base classes, while openeo-python-udf should be the actual python server implementation

well, that does not really solve your original issue I think. The problem is that openeo-python-client currently depends on openeo-udf for very little but the dependency drags in a lot of secondary unnecessary stuff. My current idea to resolve this is to reverse the dependency so that the necessary base classes are are in the python client and openeo-udf depends on that.

Ideally, openeo-python-client should have as few (lesser widespread) dependencies as possible I think.

@m-mohr
Copy link
Member Author

m-mohr commented Oct 29, 2020

I had assumed that the base udf classes (without any server stuff) would not have many (non-dev) dependencies (if any)... Then it would solve the issue, I think.
The other way round also feels a bit strange. Why would a UDF server depend on the Python client?

@jdries
Copy link
Collaborator

jdries commented Nov 27, 2020

Our backend also depends on the python client. We simply do it like that, because it's more annoying for the large group of Python client users to drag in server dependencies than the other way round. (Backend devs tend to know how to deal with dependencies, client users are researchers for whom all of this is very magical.)
Putting it in a separate repo is an option, but same argument: introduces opportunities for dependency issues, that client users will not know how to deal with. Also consider the documentation perspective: having it in the client will also result in consolidated docs.

soxofaan added a commit that referenced this issue Mar 15, 2021
…neo_udf` `DataCube`

Based on original `openeo_udf.api.datacube.DataCube` implementation
Introduces new `openeo.udf` subpackage
Eliminates `openeo_udf` dependency from `openeo.rest.conversions`

related: #159, EP-3578
soxofaan added a commit that referenced this issue Mar 15, 2021
Hierarchy now goes further than `openeo.rest` "client" exceptions

related: #159, EP-3578
soxofaan added a commit that referenced this issue Mar 15, 2021
based on `UdfData` from `openeo_udf` package
only supports `XarrayDataCube` and `StructuredData` lists
soxofaan added a commit that referenced this issue Mar 17, 2021
… on `openeo_udf` `DataCube`

Based on `openeo_udf.api.datacube.DataCube` implementation
Introduces new `openeo.udf` subpackage
Eliminates `openeo_udf` dependency from `openeo.rest.conversions`
soxofaan added a commit that referenced this issue Mar 17, 2021
Hierarchy now goes further than `openeo.rest` "client" exceptions

related: #159, EP-3578
soxofaan added a commit that referenced this issue Mar 17, 2021
based on `UdfData` from `openeo_udf` package
only supports `XarrayDataCube` and `StructuredData` lists
soxofaan added a commit that referenced this issue Mar 17, 2021
Based on `openeo_udf.api.run_code` implementation
Eliminates `openeo_udf` dependency from `openeo.rest.datacube`
soxofaan added a commit that referenced this issue Mar 17, 2021
Turned out that original tests did not test netcdf functionality properly
(no asserts were done). Failing tests are skipped for now
soxofaan added a commit that referenced this issue Mar 17, 2021
Allows to drop `--extra-index-url`again from pip install instructions
soxofaan added a commit that referenced this issue Mar 17, 2021
… on `openeo_udf` `DataCube`

Based on `openeo_udf.api.datacube.DataCube` implementation
Introduces new `openeo.udf` subpackage
Eliminates `openeo_udf` dependency from `openeo.rest.conversions`
soxofaan added a commit that referenced this issue Mar 17, 2021
Hierarchy now goes further than `openeo.rest` "client" exceptions

related: #159, EP-3578
soxofaan added a commit that referenced this issue Mar 17, 2021
based on `UdfData` from `openeo_udf` package
only supports `XarrayDataCube` and `StructuredData` lists
soxofaan added a commit that referenced this issue Mar 17, 2021
Based on `openeo_udf.api.run_code` implementation
Eliminates `openeo_udf` dependency from `openeo.rest.datacube`
soxofaan added a commit that referenced this issue Mar 17, 2021
Turned out that original tests did not test netcdf functionality properly
(no asserts were done). Failing tests are skipped for now
soxofaan added a commit that referenced this issue Apr 13, 2021
Turned out that original tests did not test netcdf functionality properly
(no asserts were done). Failing tests are skipped for now
soxofaan added a commit that referenced this issue Apr 13, 2021
Allows to drop `--extra-index-url`again from pip install instructions
soxofaan added a commit that referenced this issue Apr 13, 2021
soxofaan added a commit that referenced this issue Apr 13, 2021
…package

`execute_local_udf` had nothing to do with  `rest.DataCube`
soxofaan added a commit that referenced this issue Apr 13, 2021
…aydatacube`

Doesn't belong anymore in `openeo.rest.conversions`
soxofaan added a commit that referenced this issue Apr 13, 2021
…_to_NetCDF

Apparently these private functions are used in openeo-geopyspark-driver so we can not just remove them

Reorganized these load/save functions a bit in `XarrayIO` helper class
soxofaan added a commit that referenced this issue Apr 13, 2021
Simplified version of original `FeatureCollection` from `openeo_udf`
soxofaan added a commit that referenced this issue Apr 13, 2021
soxofaan added a commit that referenced this issue Apr 13, 2021
original implementation applied along y-axis instead of t-axis

new implementation explicitly searches for time dimension and allows input to have less or more than 3 dims (t-x-y)
soxofaan added a commit that referenced this issue Apr 13, 2021
soxofaan added a commit that referenced this issue Apr 13, 2021
…eric

It does not make sense to have a default value for callback
soxofaan added a commit that referenced this issue Apr 13, 2021
…ne to empty list

remove unnecessary magic behind the scenes
makes API more straightforward and easier to reason about
soxofaan added a commit that referenced this issue Apr 13, 2021
#159/EP-3578 related changes are large enough to justify this bump
@soxofaan
Copy link
Member

just merged #190 which solves this issue
(will be in next release 0.7.0)

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

3 participants