Hi Dong and Zhiping, Thanks for writing up the FLIP and sorry for the belated review. The FLIP looks good to me overall. Just a few minor comments / questions about WithParams.
1. Currently all the methods in the WithParams interface have a default implementation. It is not clear to me what exactly these default implementations will do. For example, in the example usage, MyParams extends WithParams and has some static Param defined, how would the default getParam() work in this case? 2. What is the difference between getParamMap() and getUserDefinedParamMap()? It looks that one of them is mutable and the other is not. Apart from that, are they going to return the same mapping? If users want to set param values, can they just call set() directly? In which case would they want to get a mutable param map and set the param values directly in that map? Thanks, Jiangjie (Becket) Qin On Thu, Sep 23, 2021 at 1:33 PM Dong Lin <lindon...@gmail.com> wrote: > Hi all, > > Zhipeng and I have created FLIP-174: Improve the WithParam interface. > Please find the FLIP wiki in the link > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=181311361 > . > > The goal of this FLIP is to improve the experience of defining, saving and > loading parameters of stages (e.g. Transformer and Estimator) in the Flink > ML repo. > > It will be really helpful if you can help review this FLIP and provide your > feedback. > > Thanks! > Dong and Zhipeng >
