You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "simsurace (via GitHub)" <gi...@apache.org> on 2023/04/06 11:07:28 UTC

[GitHub] [arrow-julia] simsurace opened a new pull request, #415: Use `getproperty` instead of type parameter to get names

simsurace opened a new pull request, #415:
URL: https://github.com/apache/arrow-julia/pull/415

   Closes #414 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-julia] ericphanson commented on pull request #415: Use `getproperty` instead of type parameter to get names

Posted by "ericphanson (via GitHub)" <gi...@apache.org>.
ericphanson commented on PR #415:
URL: https://github.com/apache/arrow-julia/pull/415#issuecomment-1498905841

   Can you add a test?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-julia] ericphanson commented on pull request #415: Use `getproperty` instead of type parameter to get names

Posted by "ericphanson (via GitHub)" <gi...@apache.org>.
ericphanson commented on PR #415:
URL: https://github.com/apache/arrow-julia/pull/415#issuecomment-1500966150

   There is a process documented partly here: https://github.com/apache/arrow-julia/tree/main/dev/release. It takes a day or two since there needs to be a vote. Can you PR a patch bump to get it started? We can ask @kou if they are willing to work on the next steps after.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-julia] ericphanson commented on pull request #415: Use `getproperty` instead of type parameter to get names

Posted by "ericphanson (via GitHub)" <gi...@apache.org>.
ericphanson commented on PR #415:
URL: https://github.com/apache/arrow-julia/pull/415#issuecomment-1498924070

   Maybe this one? https://tables.juliadata.org/dev/#Tables.table


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-julia] simsurace commented on pull request #415: Use `getproperty` instead of type parameter to get names

Posted by "simsurace (via GitHub)" <gi...@apache.org>.
simsurace commented on PR #415:
URL: https://github.com/apache/arrow-julia/pull/415#issuecomment-1501072505

   I'll do that once I get back to my computer.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-julia] ericphanson commented on a diff in pull request #415: Use `getproperty` instead of type parameter to get names

Posted by "ericphanson (via GitHub)" <gi...@apache.org>.
ericphanson commented on code in PR #415:
URL: https://github.com/apache/arrow-julia/pull/415#discussion_r1160014404


##########
test/runtests.jl:
##########
@@ -572,6 +572,17 @@ if pkgversion(ArrowTypes) >= v"2.0.2"
     @test isequal(a.x, t.x)
 end
 
+# https://github.com/apache/arrow-julia/issues/414
+df = DataFrame(("$i" => rand(1000) for i in 1:65536)...)
+Arrow.write("df.arrow", df)
+df_load = Arrow.Table("df.arrow")
+@test Tables.schema(df) == Tables.schema(df_load)
+for (col1, col2) in zip(Tables.columns(df), Tables.columns(df_load))
+    @test col1 == col2
+end
+rm("df.arrow")

Review Comment:
   We can roundtrip through serialization without actually writing to disk by using the helper `Arrow.tobuffer`:
   ```suggestion
   df_load = Arrow.Table(Arrow.tobuffer(df))
   @test Tables.schema(df) == Tables.schema(df_load)
   for (col1, col2) in zip(Tables.columns(df), Tables.columns(df_load))
       @test col1 == col2
   end
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-julia] ericphanson commented on pull request #415: Use `getproperty` instead of type parameter to get names

Posted by "ericphanson (via GitHub)" <gi...@apache.org>.
ericphanson commented on PR #415:
URL: https://github.com/apache/arrow-julia/pull/415#issuecomment-1498946848

   Ah that does sound like a bug. Well, we can add a test-only dependency on DataFrames by adding it to extras & the test target in the Project.toml


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-julia] simsurace commented on pull request #415: Use `getproperty` instead of type parameter to get names

Posted by "simsurace (via GitHub)" <gi...@apache.org>.
simsurace commented on PR #415:
URL: https://github.com/apache/arrow-julia/pull/415#issuecomment-1498912964

   Sure. What do we want to test exactly? To prevent #414 we would need a table type that allows for >65535 columns.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-julia] ericphanson merged pull request #415: Use `getproperty` instead of type parameter to get names

Posted by "ericphanson (via GitHub)" <gi...@apache.org>.
ericphanson merged PR #415:
URL: https://github.com/apache/arrow-julia/pull/415


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-julia] simsurace commented on pull request #415: Use `getproperty` instead of type parameter to get names

Posted by "simsurace (via GitHub)" <gi...@apache.org>.
simsurace commented on PR #415:
URL: https://github.com/apache/arrow-julia/pull/415#issuecomment-1500959894

   Thanks for getting this merged quickly. What is the release cycle? Can we get this released as a patch?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-julia] simsurace commented on pull request #415: Use `getproperty` instead of type parameter to get names

Posted by "simsurace (via GitHub)" <gi...@apache.org>.
simsurace commented on PR #415:
URL: https://github.com/apache/arrow-julia/pull/415#issuecomment-1498936702

   Unfortunately (and maybe this is a bug in Tables.jl) `Tables.schema(::MatrixTable)` stores the names in the type parameter, so it won't expose the problem in #414.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow-julia] simsurace commented on pull request #415: Use `getproperty` instead of type parameter to get names

Posted by "simsurace (via GitHub)" <gi...@apache.org>.
simsurace commented on PR #415:
URL: https://github.com/apache/arrow-julia/pull/415#issuecomment-1499319885

   @ericphanson I added a test as suggested by you.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org