Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to StatsBase v0.34 #59

Merged
merged 4 commits into from
Aug 17, 2023
Merged

Upgrade to StatsBase v0.34 #59

merged 4 commits into from
Aug 17, 2023

Conversation

AsafManela
Copy link
Contributor

  1. StatsBase.jl v0.34 removed some internal array types used by MLBase. This change adds a dependency to NumericalTypeAliases.jl that provides these type aliases.

JuliaStats/StatsBase.jl#840

  1. Also adding a CompatHelper action

  2. Bumps MLBase version so it can be tagged

@codecov-commenter
Copy link

codecov-commenter commented Aug 16, 2023

Codecov Report

Patch coverage: 90.00% and no project coverage change.

Comparison is base (9128e4f) 78.75% compared to head (d9ee6c2) 78.75%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #59   +/-   ##
=======================================
  Coverage   78.75%   78.75%           
=======================================
  Files           7        7           
  Lines         659      659           
=======================================
  Hits          519      519           
  Misses        140      140           
Files Changed Coverage Δ
src/MLBase.jl 100.00% <ø> (ø)
src/classification.jl 90.75% <84.21%> (ø)
src/perfeval.jl 95.32% <92.85%> (ø)
src/utils.jl 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AsafManela
Copy link
Contributor Author

Hi @ararslan , would you mind taking a look and merging? I need this for a couple of downstream packages.

@ararslan
Copy link
Member

It seems worthwhile to me to replace the uses of those type aliases with their definitions in the same manner as JuliaStats/StatsBase.jl#840 rather than adding a new dependency on a package that defines them.

@AsafManela
Copy link
Contributor Author

I guess I missed your sarcasm here JuliaStats/StatsBase.jl#840 (review)
I'll try to do that.

@ararslan
Copy link
Member

Ah, my apologies, the :trollface: in my comment you linked was intended to convey that it wasn't a serious suggestion. I can see how that could be unclear so I hope my comment hasn't been accidentally misguiding people...

@AsafManela
Copy link
Contributor Author

No worries. I made those changes. Hopefully good to go.

Copy link
Member

@ararslan ararslan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a minor tweak to compatibility bounds

Project.toml Outdated Show resolved Hide resolved
@ararslan ararslan merged commit 14bf622 into JuliaStats:master Aug 17, 2023
10 checks passed
@ararslan
Copy link
Member

Registering here: JuliaRegistries/General#89850

Thanks!

@AsafManela
Copy link
Contributor Author

Thanks for the quick turnaround!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants