Conversation
Project.toml
Outdated
| [deps] | ||
| LinearAlgebra = "37e2e46d-f89d-539d-b4ee-838fcccc9c8e" | ||
| StatsBase = "2913bbd2-ae8a-5f71-8c99-4fb6c76f3a91" | ||
| Markdown = "d6f4376e-aef5-505a-96c1-9c027394607a" |
There was a problem hiding this comment.
It is used in the docstrings for the LaTeX rendering. The code doesn't compile otherwise.
There was a problem hiding this comment.
instead of @doc doc"""...""" I believe @doc raw"""...""" does the same thing without the need of the package Markdown.
Ref: https://docs.julialang.org/en/v1/manual/documentation/#and-\\-characters-1
|
In regards to the list in the mentioned issue:
Also, your most significant change is dropping StatsBase, which again is 👍 from me. |
|
Thank you for your time reviewing this. I think that splitting this single source file into smaller digestible files is quite beneficial in the long run, particularly because I am planning to extend the interface with other concepts in the near future. I spent a lot of time trying to understand which functions in the interface were related to costs and which were not, and it would be nice if new potential contributors could just jump directly to the concept they care about like for example |
|
A minor version is needed since it's a breaking change by un-exporting the symbols, perhaps |
|
Thank you @johnnychen94 , we have that in mind. I am will keep improving the PR and then after it is merged I will bump the minor version for a new release. @joshday I've started adding back the docstrings for |
|
Another point of discussion is the |
|
Ok, I've finished this PR, and the interface is already much easier to follow. I'd would like to summarize the changes here before I go ahead and merge it tomorrow morning (BRT time zone): Organization
Breaking
Overall, I am very satisfied with this reorganization of the source, and feel ready to improve the downstream packages. In particular, after this PR is merged, I will come back to the other major refactoring in JuliaML/LossFunctions.jl#130 and rebase it to the cleaned API. Looking forward to improving the API further. |
UpdateI realized that the |
This PR addresses issue #35. It refactors the codebase so that we have a single concept per source file. It also disables name exports, which are now exported by downstream components (e.g. LossFunctions.jl).