Improvements to OrdinalEncoder, OneHotEncoder, NaiveBayes, LogisticRegression

[krstopro]

Changelog:

• Simplified OrdinalEncoder struct; it now contains only :categories field.
• Fixed a bug inside OrdinalEncoder.fit/2 and now it works with tensors of size 1.
• Removed opts as 2nd argument from OrdinalEncoder.fit_transform as it is not needed.
• Simplified OneHotEncoder struct; it now contains only :ordinal_encoder field.
• Renamed :num_classes option to :num_categories as these encoders can be used to encode features, not only target variables.
• Added argument validation.
• Removed ordinal encoding from NaiveBayes.Complement and LogisticRegression.

Fixes #290.

[josevalim]

LGTM! Perhaps we just need to remove the commented out code. :)

[krstopro]

LGTM! Perhaps we just need to remove the commented out code. :)

Yeah, agreed. :)

I would honestly replace the following line https://github.com/elixir-nx/scholar/blob/main/lib/scholar/preprocessing.ex#L179
with

tensor
|> Nx.new_axis(1)
|> Nx.broadcast({num_samples, num_classes})
|> Nx.equal(Nx.iota({num_samples, num_categories}, axis: 1))

i.e. remove the ordinal encoding that is performed as part of OneHotEncoder and assume tensor contains values between 0 and num_categories - 1. These few lines are used over and over again in Scholar (e.g. NaiveBayes, LogisticRegression, etc.).

[josevalim]

I would honestly replace the following line https://github.com/elixir-nx/scholar/blob/main/lib/scholar/preprocessing.ex#L179

I think the preprocessing function should be the same as the module that we invoke (we may remove the functions altogether). But I agree we should probably remove the ordinal encoding from one hot encoding preprocessor module.

[krstopro]

I would honestly replace the following line https://github.com/elixir-nx/scholar/blob/main/lib/scholar/preprocessing.ex#L179

I think the preprocessing function should be the same as the module that we invoke (we may remove the functions altogether). But I agree we should probably remove the ordinal encoding from one hot encoding preprocessor module.

Very well, we can have a separate pull request for that.

[josevalim]

Definitely, so it all looks good to me!