+1. This new change makes sense to me, especially if we want to add new merge policies in the future by modularizing each merge policy.
Best regards, Chen Luo On Tue, Jun 18, 2019 at 1:46 PM Mike Carey <[email protected]> wrote: > This sounds like a good evolutionary change to me - others' thoughts? > > On 6/18/19 10:09 AM, Merlin Mao wrote: > > Currently all MergePolicy property names and types are defined in > > DatasetDeclParametersUtil for validation. It would be better to move them > > back to the MergePolicy or MergePolicyFactory for mainly 3 reasons: > > > > 1. Adding a new MergePolicy or modifying an existing MergePolicy can be > > more difficult. Now you have to register the factory, and register the > > properties. > > > > 2. It is possible some policies use the same property name but with > > different constraint. For example, for one policy, "num-components" may > > allow any integer at least 1, while another policy only allows any > integer > > at least 2. Some property may use number type while in some policy the > same > > property may allow string. > > > > 3. In the current validation, there is no way to have optional property > or > > property with default value, which means certain properties can be > omitted > > when creating datasets. > > > > If policy property validation is done in the policy itself, it gives more > > flexibility to design new policies and improve the current policies. > > >
