[mlpack-git] [mlpack/mlpack] DatasetMapper & Imputer (#694)

Tham notifications at github.com
Sat Jul 2 21:11:38 EDT 2016


> @@ -96,7 +96,11 @@ bool Load(const std::string& filename,
>            arma::Mat<eT>& matrix,
>            DatasetMapper<PolicyType>& info,
>            const bool fatal = false,
> -          const bool transpose = true);
> +          const bool transpose = true)
> +{
> +  PolicyType policy;
> +  return Load(filename, matrix, info, policy, fatal, transpose);

Since you already provide an api to access the policy of the DatasetMapper, I think we could remove another Load function which allow user to pass the policy. This would make the api more consistent.

You can create the DatasetMapper in the Load function without the policy parameters pass

`info = info = DatasetMapper<PolicyType>(info.Policy(), cols);`

By this way users only need to store the state in their DatasetMapper only, I think this is less confusing.Else the users may think 
"I already store my policy in the DatasetMapper, why should I pass the policy into the Load function again?"

If you want to allow the users to get/set their policy states, you can add two api to the DatasetMapper

```
PolicyType const& Policy() const
{
  return policy;
}
void Policy(PolicyType value)
{
  policy = std::move(value);
}
```

---
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/mlpack/mlpack/pull/694/files/a340f69411b6a6d16106023e4bbcceb9d4688322..21d94c04652e8faadd5e8991103a3b73b4c81033#r69384323
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mailman.cc.gatech.edu/pipermail/mlpack-git/attachments/20160702/6e4a4967/attachment.html>


More information about the mlpack-git mailing list