Open-EO / openeo-processes

Interoperable processes for openEO's big Earth observation cloud processing.
https://processes.openeo.org
Apache License 2.0
49 stars 15 forks source link

Date replace and Date get #433

Open PondiB opened 1 year ago

PondiB commented 1 year ago

Implementation for issue #254 closes #254

PondiB commented 10 months ago

Not a big fan of the _component suffix, otherwise just added a couple of minor comments to clean-up the descriptions. Overall very solid proposals, I think.

Okay

Has this been tested against common date-time libraries, especially the ones that are mentioned in the implementation guide? It would be good to know whether all the edge cases have been found, documented and been tested with at least the 4 libraries mentioned there (this is the reason for the change request).

Not really. What implementation guide?

m-mohr commented 10 months ago

What implementation guide?

https://github.com/Open-EO/openeo-processes/blob/master/meta/implementation.md

soxofaan commented 10 months ago

Not a big fan of the _component suffix,

You're not a fan of _component as a suffix, or any suffix at all? Do you have other suggestions?

m-mohr commented 10 months ago

You're not a fan of _component as a suffix, or any suffix at all? Do you have other suggestions?

date_get and date_replace seem fine to me, but I'm not the dictator so if many prefer the _component suffix, I'm fine with it. I don't have a good alternative at hand (maybe "part"), I also don't think we hve anything comparable right now, which we can borrow from.

soxofaan commented 10 months ago

the closest comparable process is array_element I think, so date_element could make sense too

PondiB commented 9 months ago

date_replace_component and date_get_component is more intuitive to me than date_replace, date_get and date_element.

PondiB commented 8 months ago

@m-mohr , if there is no further review of this then kindly approve.

PondiB commented 8 months ago

@soxofaan 👆🏾

PondiB commented 8 months ago

no further remarks from me, we already iterated enough on this on I guess :)

Sure, thanks.

m-mohr commented 8 months ago

@PondiB You did never followed-up on the comment with regards to extending the implementation guide. Any updates on that?

PondiB commented 7 months ago

Did a quick review. See comments above. Also, the implementation guide on what libraries support exactly this should be added (compare to the other date processes).

Cool, thanks. I'll address them later.