Skip to content

Update SolverBenchmark for DataFrames v.0.21 #56

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

Update SolverBenchmark for DataFrames v.0.21 #56

wants to merge 5 commits into from

Conversation

amontoison
Copy link
Member

@amontoison amontoison commented May 14, 2020 •
edited
Loading

supersedes #55

@abelsiqueira
Copy link
Member

The "^0.20" means [0.20.0, 0.21.0) and "=0.20" means only "0.20.0" (ref: https://julialang.github.io/Pkg.jl/v1/compatibility/). The current "^0.20" is already and upper bound.

@amontoison
Copy link
Member Author

amontoison commented May 14, 2020 •
edited
Loading

Thanks Abel!
The problem is that the last release was done without #37. The Project.toml doesn't need to be updated.

@abelsiqueira
Copy link
Member

Version 0.2.0 should have Project.toml with upper bounds.

@abelsiqueira
Copy link
Member

https://github.com/JuliaSmoothOptimizers/SolverBenchmark.jl/releases/tag/v0.2.0

@amontoison
Copy link
Member Author

Hum, I don't understand why I have DataFrames v0.21 installed.

@amontoison
Copy link
Member Author

amontoison commented May 14, 2020 •
edited
Loading

Version 0.2.0 has a Project.toml with upper bounds but it's not the case for the version 0.1.0. The solution found by Pkg was to downgrade SolverBenchmarks.
solverbenchmarks

@abelsiqueira
Copy link
Member

I guess it has to decide which package will be downgraded, and it chose SolverBenchmark. Not much we can do except update to handle DataFrames 0.21.

@amontoison amontoison changed the title Limit the version of DataFrames Update SolverBenchmark for DataFrames v.0.21 May 15, 2020
@amontoison
Copy link
Member Author

amontoison commented May 15, 2020 •
edited
Loading

flag became status in some tests.Is it a modification of the DataFrames package or I forgot to update something here?

amontoison
amontoison commented May 15, 2020
View reviewed changes
src/join.jl Outdated
c in invariant_cols && return c
sc = haskey(hdr_override, c) ? hdr_override[c] : string(c)
Symbol(c) in invariant_cols && return c
sc = haskey(hdr_override, c) ? hdr_override[c] : c
Copy link
Member Author

Choose a reason for hiding this comment

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

haskey(hdr_override, Symbol(c))

@amontoison
Copy link
Member Author

I don't understand why PkgBenchmark says that the package is dirty in the tests...

dpo
dpo reviewed May 16, 2020
View reviewed changes
Project.toml
@@ -16,11 +16,12 @@ Plots = "91a5bcdd-55d7-5caf-9e0b-520d859cae80"
PrettyTables = "08abe8d2-0d0c-5749-adfa-8a2ac140af0d"
Printf = "de0858da-6303-5e67-8744-51eddeeeb8d7"
Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
SparseArrays = "2f01184e-e22b-5df5-ae63-d93ebab69eaf"
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need SparseArrays and we didn't need it before?

Copy link
Member Author

Choose a reason for hiding this comment

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

I suppose that they fixed some type piracy with the release 1.4.1 : JuliaLang/julia#30945 or JuliaLang/julia#35899.

@dpo
Copy link
Member

dpo commented May 16, 2020

I don't understand why PkgBenchmark says that the package is dirty in the tests...

Try adding benchmark/*.json to .gitignore. I'm not sure why it wasn't an issue before.

@dpo
Copy link
Member

dpo commented May 17, 2020

Seems like PkgBenchmark is leaving more stuff around. Add a git status to .travis.yml to debug.

@amontoison
Copy link
Member Author

It should work now. When the package was cloned, the Project.toml was regenerated differently...

@abelsiqueira
Copy link
Member

Solving manually.

@abelsiqueira
Copy link
Member

Merged manually: f57c802
Thanks, @amontoison

dpo added a commit that referenced this pull request Jun 13, 2020
@dpo
fixes following #56 and #57
5c3cc94
dpo added a commit that referenced this pull request Jun 13, 2020
@dpo
fixes following #56 and #57
9110199
dpo added a commit that referenced this pull request Jun 14, 2020
@dpo
fixes following #56 and #57
762112f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dpo dpo dpo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@amontoison @abelsiqueira @dpo