Skip to content
This repository has been archived by the owner on Aug 12, 2022. It is now read-only.

Update to latest changes in DataFrames #12808

Merged
merged 1 commit into from
Jan 10, 2018
Merged

Conversation

andreasnoack
Copy link
Member

With version 0.11.4 of DataFrames this version of RDatasets works again

@nalimilan
Copy link
Member

Funny, I've just pushed a branch to do the same thing. I think you can drop the DataArrays dependency altogether since it's not used at all in RDatasets. Also, I wouldn't add an upper bound on DataFrames 1.0: if we do that, we should apply that change to all packages. Doing this only for one package can only create confusion.

@nalimilan
Copy link
Member

Cc: @randyzwitch

@andreasnoack
Copy link
Member Author

I think we should start to add upper bounds on major versions generally such that we don't need to do it retrospectively in METADATA.

@nalimilan
Copy link
Member

I think we should start to add upper bounds on major versions generally such that we don't need to do it retrospectively in METADATA.

I think Pkg3 has a mechanism to handle this (e.g. JuliaLang/Juleps#3). In the meantime, better only apply minimal changes.

@randyzwitch
Copy link
Contributor

Thanks for pinging me on this. This change makes the tests for the current RDatasets work, without needing the CSV fixes? Or is this PR JuliaStats/RDatasets.jl#48 separately needed?

@nalimilan
Copy link
Member

The tests already work with DataFrames 0.11.4, this PR just allows the package manager to upgrade DataFrames to 0.11.4 when RDatasets is installed. JuliaStats/RDatasets.jl#48 will still be needed to silence the deprecation warnings (and to ensure the package will work once deprecations are removed from DataFrames).

@randyzwitch
Copy link
Contributor

So do I need to tag a new version of RDatasets with REQUIRES DataFrames 0.11, to override the current METADATA cap of 0.10?

@nalimilan
Copy link
Member

It would make sense to update REQUIRE in the RDatasets repo to match the one in this PR, but no need to tag a new release.

@randyzwitch
Copy link
Contributor

Sorry, I didn't look at what this PR was actually doing. Now I see that it's fixing RDatasets versioning on METADATA, not changing DataFrames.jl. Got it now.

@andreasnoack
Copy link
Member Author

So do I need to tag a new version of RDatasets with REQUIRES DataFrames 0.11, to override the current METADATA cap of 0.10?

@randyzwitch No, it shouldn't be necessary to tag RDatasets now. You can just wait until JuliaStats/RDatasets.jl#48 has been merged. However, Before tagging a new version you should add DataFrames 0.11.4 1.0.0 to the REQUIRE file.

@nalimilan The current approach where the person who happens to tag a package which other depends on has to add (possibly unnecessary) upper bounds on all the dependent packages or check all the packages one-by-one is so broken that anything else is better than that. There is almost zero chance that this 1.0.0 upper bound will have an effect though.

@nalimilan
Copy link
Member

@nalimilan The current approach where the person who happens to tag a package which other depends on has to add (possibly unnecessary) upper bounds on all the dependent packages or check all the packages one-by-one is so broken that anything else is better than that. There is almost zero chance that this 1.0.0 upper bound will have an effect though.

I agree on the goal, but please apply this to all DataFrames dependency, or to none. Having spent a full afternoon on adding these bounds, I'm aware the current system is far from ideal, but let's not make it even less consistent.

With version 0.11.4 of `DataFrames` this version of `RDatasets` works again.
Also remove dependency on DataArrays since RDatasets does not use it at all.
@nalimilan nalimilan force-pushed the andreasnoack-patch-1 branch from c219a00 to 20b2c49 Compare January 10, 2018 10:56
@nalimilan
Copy link
Member

@andreasnoack I've pushed changes, is that OK with you?

@nalimilan nalimilan merged commit 832aa4f into metadata-v2 Jan 10, 2018
@nalimilan nalimilan deleted the andreasnoack-patch-1 branch January 10, 2018 13:21
@randyzwitch
Copy link
Contributor

Thanks everyone

randyzwitch added a commit to JuliaStats/RDatasets.jl that referenced this pull request Jan 11, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants