Upgrade to Julia 1.0 #104

bkamins · 2018-08-18T08:01:30Z

Prepare a for a new release of DataFramesMeta.jl working on Julia 1.0.

bkamins · 2018-08-18T09:24:22Z

This is WIP. Depends on fixing DataArrays for Julia 1.0 first. Also some macros like @by_row are not fixed yet (I am trying to work out why row is not added in calls by byrow_replce in Julia 1.0 so any hints are appreciated).

bkamins · 2018-08-18T12:38:25Z

Now it is good for a review. I have it working under 1.0 with all core functionality.

I did not do support (and they are commented out in tests) for two things:

DataArrays package dependencies (in tests) - do we need to test against it (I do not know how much this package is used, but for sure it is not updated for Julia 1.0 and it will require many changes to provide support);
CompositeDataFrame which uses eval in other module, which is problematic AFAIK (I am not sure if support for CompositeDataFrame is needed as it does not keep up with changes in DataFrames.jl functionality anyway and we have JuliaDB that provides type stable column access).

Therefore, I would drop those two things (the second is more problematic as it is breaking in functionality - but maybe there is a simple and safe fix for this?).

@nalimilan Any opinions about the best approach? I would want to tag a release of DataFramesMeta that would support Julia 1.0.

bkamins · 2018-08-18T12:39:31Z

CC @tshort (as this is a major discussion point what I propose)

tshort · 2018-08-18T12:49:46Z

I'm good with dropping those two items. Thanks for all the work here!

bkamins · 2018-08-18T15:26:14Z

Thank you for a prompt response. After thinking about it I have made two additional commits that clean up everything of DataArrays and CompositeDataFrame. The first one cleans the code, and the second one is an update of the documentation.

The rationale is that DataArrays are an unnecessary dependency in tests, while CompositeDataFrame is an independent code from core macro-targeted functionality of the package. I think it is better - if someone wants to maintain CompositeDataFrame to keep it in a separate package, as then DataFramesMeta will be easier to maintain (and they do not work "as is" anyway on Julia 1.0 so there is some work to be done to port them).

If this proposal would be acceptable I would recommend merging the PR and tagging a release.

tshort · 2018-08-18T19:00:09Z

I'm good with deleting all the CI tests on nightly, including Travis.

bkamins · 2018-08-18T19:22:42Z

I would leave them for now if you are OK with this as I expect that soon some minor bug release of Julia 1.0 might come out.

tshort · 2018-08-18T19:43:24Z

That's fine.

pdeffebach · 2018-08-18T22:19:24Z

If this update is going to have breaking changes in it, should we consider #100. Since semantic versioning is so strict now its worth going over proposed breaking changes.

tshort · 2018-08-18T22:54:41Z

I'm good with #100 and a semantic version bump.

pdeffebach · 2018-08-18T23:52:45Z

Cool! also keep in mind that #101 is essentially a bug-fix (fixing some edge cases that throw errors with @transdorm), so it can be added in at any time.

tshort · 2018-08-18T23:55:13Z

Sorry, I'm a bit lost in the #101 thread :).

pdeffebach · 2018-08-19T00:16:21Z

Yup it got lost in the weeds a bit. A rebase might be in order for it since I think its trying to do too much.

bkamins · 2018-08-19T05:37:57Z

@pdeffebach I agree that it would be good to have #100 (I left a small comment there) and #101 in the release.

I would propose the following process:

merge Upgrade to Julia 1.0 #104 as it transitions to Julia 1.0 and has massive changes in many places (I would merge it on 2018-09-20 to wait a bit and see if there are any comments/objections to the changes);
then you rebase Rename _I_ to cols to be consistent with others #100 and Change the way grouped transforms work #101 and add whatever changes are needed so that we can see that CI goes cleanly (I guess this should be possible to be fast 😄 and we could merge them fast next)
we tag a new release.

nalimilan · 2018-08-19T08:42:12Z

.travis.yml

@@ -1,9 +1,10 @@
 language: julia
 julia:
-  - 0.6
+  - 0.7


Maybe also add 1.0?

nalimilan · 2018-08-19T08:43:20Z

README.md

@@ -5,7 +5,7 @@
 [![Travis](https://travis-ci.org/JuliaStats/DataFramesMeta.jl.svg?branch=master)](https://travis-ci.org/JuliaStats/DataFramesMeta.jl)
 [![AppVeyor](https://ci.appveyor.com/api/projects/status/github/juliastats/dataframesmeta.jl?branch=master&svg=true)](https://ci.appveyor.com/project/tshort/dataframesmeta-jl/branch/master)

-Metaprogramming tools for DataFrames and Associative objects.
+Metaprogramming tools for DataFrames and AbstractDict objects.


Could add backticks while you're at it.

nalimilan · 2018-08-19T08:44:29Z

README.md

 the original `df` is not affected. Here is an example where two new columns are
 added:

 ```julia
 df = DataFrame(A = 1:3, B = [2, 1, 2])
 df2 = @byrow! df begin
    @newcol colX::Array{Float64}
-    @newcol colY::DataArray{Int}
+    @newcol colY::Array{Int}


Maybe use Union{Int,Missing} to illustrate that missing values are supported?

fixed and added a case with missing to show that it actually works.

nalimilan · 2018-08-19T08:45:08Z

README.md

@@ -155,28 +149,10 @@ functions.
    @select           select           Select


-Chaining operations is a useful way to manipulate data. There are


Is this no longer supported? I haven't followed the updates on this.

It does not work on Julia 1.0 due to Lazy.jl issues. The relevant test are also commented out. When it starts working again it should be reinserted after tests are uncommented (I have added a note there).

nalimilan · 2018-08-19T08:46:07Z

REQUIRE

-Compat 0.57.0
+julia 0.7
+DataFrames 0.13
+Compat 1.0


Can't we get rid of the dependency on Compat?

nalimilan · 2018-08-19T08:47:53Z

appveyor.yml

-  - JULIA_URL: "https://julialangnightlies-s3.julialang.org/bin/winnt/x86/julia-latest-win32.exe"
-  - JULIA_URL: "https://julialangnightlies-s3.julialang.org/bin/winnt/x64/julia-latest-win64.exe"
+  - julia_version: 0.7
+  - julia_version: 1


I'd say better test 1.0, since we already have nightly to test recent versions. That way we ensure we still really support 1.0 (I know there's a debate going on about that).

nalimilan · 2018-08-19T08:50:12Z

test/data.table.timings.jl

@@ -50,21 +50,6 @@ DMA = DataFrame(
  v3 =  Array{Union{Float64, Missing}}(rand(N))                            # numeric e.g. 23.5749
 );

-# DataArray version


Better keep this to benchmark Array{Union{T, Missing}}?

We have a benchmark for Union{T, Missing} in DataFrame named DMA.

bkamins · 2018-08-21T19:02:17Z

@nalimilan if you approve this PR can you merge it so that #100 can be rebased? After #100 is merged I think we should tag a release. Thanks!

nalimilan · 2018-08-22T11:34:20Z

Sure!

bkamins · 2018-08-22T12:13:53Z

Thanks! 😄

cstjean · 2018-08-22T15:26:49Z

REQUIRE

@@ -1,3 +1,2 @@
-julia 0.6
-DataFrames 0.11
-Compat 0.57.0


This PR takes out Compat, but src/DataFramesMeta.jl still imports it.

#107 fixes this

bkamins added 2 commits August 18, 2018 10:00

upgrade Julia 1.0

0912bc0

additional fixes

e4ce6e9

bkamins changed the title ~~Upgrade to Julia 1.0~~ WIP: Upgrade to Julia 1.0 Aug 18, 2018

bkamins added 2 commits August 18, 2018 12:23

fix @newCol

5786f8a

many fixes for 1.0

ddd3799

bkamins changed the title ~~WIP: Upgrade to Julia 1.0~~ Upgrade to Julia 1.0 Aug 18, 2018

bkamins added 2 commits August 18, 2018 17:00

Removed DataArrays dependencies in tests and CompositeDataFrame type

e31f9b3

major documentation update

8e4eea3

tshort approved these changes Aug 18, 2018

View reviewed changes

update appveyor.yml

3293938

nalimilan reviewed Aug 19, 2018

View reviewed changes

cleanup after a review

ee2effa

nalimilan merged commit e309357 into JuliaData:master Aug 22, 2018

cstjean reviewed Aug 22, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to Julia 1.0 #104

Upgrade to Julia 1.0 #104

bkamins commented Aug 18, 2018

bkamins commented Aug 18, 2018

bkamins commented Aug 18, 2018

bkamins commented Aug 18, 2018

tshort commented Aug 18, 2018

bkamins commented Aug 18, 2018

tshort commented Aug 18, 2018

bkamins commented Aug 18, 2018

tshort commented Aug 18, 2018

pdeffebach commented Aug 18, 2018

tshort commented Aug 18, 2018

pdeffebach commented Aug 18, 2018

tshort commented Aug 18, 2018

pdeffebach commented Aug 19, 2018

bkamins commented Aug 19, 2018

nalimilan Aug 19, 2018

bkamins Aug 21, 2018

nalimilan Aug 19, 2018

bkamins Aug 21, 2018

nalimilan Aug 19, 2018

bkamins Aug 21, 2018

nalimilan Aug 19, 2018

bkamins Aug 21, 2018

nalimilan Aug 19, 2018

bkamins Aug 21, 2018

nalimilan Aug 19, 2018

bkamins Aug 21, 2018

nalimilan Aug 19, 2018

bkamins Aug 21, 2018

bkamins commented Aug 21, 2018

nalimilan commented Aug 22, 2018

bkamins commented Aug 22, 2018

cstjean Aug 22, 2018

bkamins Aug 22, 2018

		@@ -155,28 +149,10 @@ functions.
		@select select Select


		Chaining operations is a useful way to manipulate data. There are

Upgrade to Julia 1.0 #104

Upgrade to Julia 1.0 #104

Conversation

bkamins commented Aug 18, 2018

bkamins commented Aug 18, 2018

bkamins commented Aug 18, 2018

bkamins commented Aug 18, 2018

tshort commented Aug 18, 2018

bkamins commented Aug 18, 2018

tshort commented Aug 18, 2018

bkamins commented Aug 18, 2018

tshort commented Aug 18, 2018

pdeffebach commented Aug 18, 2018

tshort commented Aug 18, 2018

pdeffebach commented Aug 18, 2018

tshort commented Aug 18, 2018

pdeffebach commented Aug 19, 2018

bkamins commented Aug 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bkamins commented Aug 21, 2018

nalimilan commented Aug 22, 2018

bkamins commented Aug 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment