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

run_udf(_externally): don't mention raster-cube in the schemas #286

Merged
merged 2 commits into from
Oct 25, 2021

Conversation

m-mohr
Copy link
Member

@m-mohr m-mohr commented Oct 18, 2021

See issue #285 for context.

This was discussed today with some of you already and this is the resulting PR.

@m-mohr m-mohr added this to the 1.2.0 milestone Oct 18, 2021
Copy link
Member

@soxofaan soxofaan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks fine to me,
just a minor note

proposals/run_udf_externally.json Outdated Show resolved Hide resolved
run_udf.json Outdated Show resolved Hide resolved
"description": "Any data type."
}
]
"description": "The data processed by the UDF. The returned value can be of any data type and is exactly what the UDF code returns.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The returned value can be of any data type and is exactly what the UDF code returns.
Is that true? Are there any backends that support returning anything else than a Raster?
Especially interesting when transmitting "trained model objects" back and forth.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure whether I agree here, but this indeed should be phrased more carefully as we indeed also thing about "helper" signatures that users can implement, which would return something different... Hard to document this properly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"description": "The data processed by the UDF. The returned value can be of any data type and is exactly what the UDF code returns.",
"description": "The data processed by the UDF. The returned value can in principle be of any data type, but it depends on what is returned by the UDF code. Please see the implemented UDF interface for details.",

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UDF interface here refers to the "UDF signature" implemented as there are a couple of helpers that make analysis easier for the user, but influence what is returned.

Copy link
Member

@przell przell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've commented directly. Generally also like this approach!

@m-mohr m-mohr merged commit 6ec1848 into draft Oct 25, 2021
@m-mohr m-mohr deleted the issue-285 branch October 25, 2021 10:16
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.

3 participants