MLJModels.jl icon indicating copy to clipboard operation
MLJModels.jl copied to clipboard

Cleanup - Consistent code style

Open darrencl opened this issue 5 years ago • 2 comments

As per the contributing guidelines, we should adhere to blue-style.

However, some parts of the code seems to not following the guideline, e.g.

https://github.com/alan-turing-institute/MLJModels.jl/blob/885bcc0a33a86fe2ee46340f20a9d99c351b1251/src/builtins/Transformers.jl#L252-L253

Which according to bluestyle, this should be

function MLJBase.inverse_transform(
    transformer::UnivariateDiscretizer, result , k::Integer
) 

If the contributors are expected to follow the guidelines, the code will be inconsistent. For that reason, I think we might need to do clean up at some stage to align with the style.

Maybe the easiest is to run JuliaFormatter? With this, the future contributors can easily run the formatter as well. Although, I think at some parts the formatter still doesn't satisfy BlueStyle? (See this issue)

darrencl avatar Mar 06 '20 23:03 darrencl

Ha I didn't know of JuliaFormatter, it could be a good idea, do you have experience with it?

Also generally in terms of formatting, I think it would be great to generally have a consistent style but I think Anthony would agree with me that we also don't want to be super strict with it as long as the code is readable.

But otherwise I'm for it, I'll be happy to review a PR, possibly I would suggest to do this in chunks starting small to begin with to see if there's anything we really don't like and adjusting accordingly 😄

Thanks!

tlienart avatar Mar 07 '20 14:03 tlienart

@tlienart I just tried it last week so haven't had a lot of experience with it. However, so far this seems promising given this is width-sensitive formatter and inspired by other formatters, such as black (python), which I used a lot in the past.

For sure, I can submit a PR starting with formatting 1 file and we can look how's the result. :smile:

darrencl avatar Mar 08 '20 09:03 darrencl