arrow icon indicating copy to clipboard operation
arrow copied to clipboard

GH-39182: [C++] Remove Legacy CastTo function

Open llama90 opened this issue 1 year ago • 8 comments

Rationale for this change

Remove legacy CastTo function.

What changes are included in this PR?

  • Remove legacy CastTo function.
  • Replace CastTo to Cast function for ToString
  • [ ] Update formating for list type

Are these changes tested?

Yes. It is passed by existing test cases.

Are there any user-facing changes?

No.

  • Closes: #39182

llama90 avatar Dec 12 '23 08:12 llama90

:warning: GitHub issue #39182 has been automatically assigned in GitHub to PR creator.

github-actions[bot] avatar Dec 12 '23 08:12 github-actions[bot]

@bkietz Hello. Is this your intention in creating the issue?

Also, thank you for creating the issue. It helped me spot and remove some code I missed.

llama90 avatar Dec 12 '23 08:12 llama90

@bkietz I got it.

llama90 avatar Dec 14 '23 06:12 llama90

:warning: GitHub issue #39182 has been automatically assigned in GitHub to PR creator.

github-actions[bot] avatar Jan 08 '24 01:01 github-actions[bot]

For recoding

glib test log

test log
 BUNDLE_GEMFILE=../c_glib/Gemfile bundle exec ../c_glib/test/run-test.sh
[72/81] Generating arrow-flight-glib/ArrowFlight-1.0.gir with a custom command (wrapped by meson to set env)
Package 'arrow', required by 'arrow-glib-uninstalled', not found

[74/81] Generating gandiva-glib/Gandiva-1.0.gir with a custom command (wrapped by meson to set env)
Package 'arrow', required by 'arrow-glib-uninstalled', not found

[76/81] Generating parquet-glib/Parquet-1.0.gir with a custom command (wrapped by meson to set env)
Package 'arrow', required by 'arrow-glib-uninstalled', not found

[78/81] Generating arrow-dataset-glib/ArrowDataset-1.0.gir with a custom command (wrapped by meson to set env)
Package 'arrow', required by 'arrow-glib-uninstalled', not found

[80/81] Generating arrow-flight-sql-glib/ArrowFlightSQL-1.0.gir with a custom command (wrapped by meson to set env)
Package 'arrow', required by 'arrow-glib-uninstalled', not found

[81/81] Generating arrow-flight-sql-glib/ArrowFlightSQL-1.0.typelib with a custom command
Loaded suite test
Started
O
==================================================================================================================================================================================================
Omission: Arrow CUDA is required [test_context(TestCUDA::Buffer)]
/Users/lama/workspace/arrow-latest/c_glib/test/test-cuda.rb:23:in `setup'
==================================================================================================================================================================================================
O
==================================================================================================================================================================================================
Omission: Arrow CUDA is required [test_copy(TestCUDA::Buffer)]
/Users/lama/workspace/arrow-latest/c_glib/test/test-cuda.rb:23:in `setup'
==================================================================================================================================================================================================
O
==================================================================================================================================================================================================
Omission: Arrow CUDA is required [test_export(TestCUDA::Buffer)]
/Users/lama/workspace/arrow-latest/c_glib/test/test-cuda.rb:23:in `setup'
==================================================================================================================================================================================================
O
==================================================================================================================================================================================================
Omission: Arrow CUDA is required [test_default(TestCUDA::Buffer::#read_record_batch)]
/Users/lama/workspace/arrow-latest/c_glib/test/test-cuda.rb:23:in `setup'
==================================================================================================================================================================================================
O
==================================================================================================================================================================================================
Omission: Arrow CUDA is required [test_options(TestCUDA::Buffer::#read_record_batch)]
/Users/lama/workspace/arrow-latest/c_glib/test/test-cuda.rb:23:in `setup'
==================================================================================================================================================================================================
O
==================================================================================================================================================================================================
Omission: Arrow CUDA is required [test_new(TestCUDA::BufferInputStream)]
/Users/lama/workspace/arrow-latest/c_glib/test/test-cuda.rb:23:in `setup'
==================================================================================================================================================================================================
O
==================================================================================================================================================================================================
Omission: Arrow CUDA is required [test_buffer(TestCUDA::BufferOutputStream)]
/Users/lama/workspace/arrow-latest/c_glib/test/test-cuda.rb:23:in `setup'
==================================================================================================================================================================================================
O
==================================================================================================================================================================================================
Omission: Arrow CUDA is required [test_new(TestCUDA::BufferOutputStream)]
/Users/lama/workspace/arrow-latest/c_glib/test/test-cuda.rb:23:in `setup'
==================================================================================================================================================================================================
O
==================================================================================================================================================================================================
Omission: Arrow CUDA is required [test_allocated_size(TestCUDA::Context)]
/Users/lama/workspace/arrow-latest/c_glib/test/test-cuda.rb:23:in `setup'
==================================================================================================================================================================================================
O
==================================================================================================================================================================================================
Omission: Arrow CUDA is required [test_new(TestCUDA::HostBuffer)]
/Users/lama/workspace/arrow-latest/c_glib/test/test-cuda.rb:23:in `setup'
==================================================================================================================================================================================================
F
==================================================================================================================================================================================================
Failure: test_to_s(TestDayTimeIntervalScalar)
/Users/lama/workspace/arrow-latest/c_glib/test/test-day-time-interval-scalar.rb:41:in `test_to_s'
     38:   end
     39:
     40:   def test_to_s
  => 41:     assert_equal("3d100ms", @scalar.to_s)
     42:   end
     43:
     44:   def test_value
<"3d100ms"> expected but was
<"[\n" + "  3d100ms\n" + "]">

diff:
+ [
?   3d100ms
+ ]
==================================================================================================================================================================================================
F
==================================================================================================================================================================================================
Failure: test_to_s(TestDenseUnionScalar)
/Users/lama/workspace/arrow-latest/c_glib/test/test-dense-union-scalar.rb:52:in `test_to_s'
     49:   end
     50:
     51:   def test_to_s
  => 52:     assert_equal("union{number: int8 = -29}", @scalar.to_s)
     53:   end
     54:
     55:   def test_value
<"union{number: int8 = -29}"> expected but was
<"-- is_valid: all not null\n" +
"-- type_ids:   [\n" +
"    2\n" +
"  ]\n" +
"-- value_offsets:   [\n" +
"    0\n" +
"  ]\n" +
"-- child 0 type: int8\n" +
"  [\n" +
"    -29\n" +
"  ]\n" +
"-- child 1 type: string\n" +
"  []">

diff:
+ -- is_valid: all not null
+ -- type_ids:   [
+     2
+   ]
+ -- value_offsets:   [
+     0
+   ]
? un   ion{numb er: int8 = -29}
? -- ch ld 0 typ
? ?? ??????? -      -------
+   [
+     -29
+   ]
+ -- child 1 type: string
+   []
==================================================================================================================================================================================================
O
==================================================================================================================================================================================================
Omission: gobject-introspection gem doesn't support implementing methods for GLib object yet [test_extension_name(TestExtensionDataType)]
/Users/lama/workspace/arrow-latest/c_glib/test/test-extension-data-type.rb:66:in `test_extension_name'
==================================================================================================================================================================================================
O
==================================================================================================================================================================================================
Omission: gobject-introspection gem doesn't support implementing methods for GLib object yet [test_to_s(TestExtensionDataType)]
/Users/lama/workspace/arrow-latest/c_glib/test/test-extension-data-type.rb:54:in `test_to_s'
==================================================================================================================================================================================================
O
==================================================================================================================================================================================================
Omission: gobject-introspection gem doesn't support implementing methods for GLib object yet [test_wrap_array(TestExtensionDataType)]
/Users/lama/workspace/arrow-latest/c_glib/test/test-extension-data-type.rb:72:in `test_wrap_array'
==================================================================================================================================================================================================
O
==================================================================================================================================================================================================
Omission: gobject-introspection gem doesn't support implementing methods for GLib object yet [test_wrap_chunked_array(TestExtensionDataType)]
/Users/lama/workspace/arrow-latest/c_glib/test/test-extension-data-type.rb:88:in `test_wrap_chunked_array'
==================================================================================================================================================================================================
F
==================================================================================================================================================================================================
Failure: test_to_s(TestListScalar)
/Users/lama/workspace/arrow-latest/c_glib/test/test-list-scalar.rb:44:in `test_to_s'
     41:   end
     42:
     43:   def test_to_s
  => 44:     assert_equal("list<item: list<value: int8>>[list<value: int8>[1, 2, 3]]",
     45:                  @scalar.to_s)
     46:   end
     47:
<"list<item: list<value: int8>>[list<value: int8>[1, 2, 3]]"> expected but was
<"[\n" + "  [\n" + "    1,\n" + "    2,\n" + "    3\n" + "  ]\n" + "]">

diff:
+ [
+   [
+     1,
? list<item: list<value: int8>>[list<value: int8>[1, 2, 3]]
?
? ??????????????????????????????????????????????????                                                  ----
+     3
+   ]
+ ]
==================================================================================================================================================================================================
F
==================================================================================================================================================================================================
Failure: test_to_s(TestMapScalar)
/Users/lama/workspace/arrow-latest/c_glib/test/test-map-scalar.rb:59:in `test_to_s'
     56:   end
     57:
     58:   def test_to_s
  => 59:     assert_equal(<<-MAP.strip, @scalar.to_s)
     60: map<string, int8>[{key:string = hello, value:int8 = 1}, {key:string = world, value:int8 = 2}]
     61:                  MAP
     62:   end
<"map<string, int8>[{key:string = hello, value:int8 = 1}, {key:string = world, value:int8 = 2}]"> expected but was
<"[\n" +
"  keys:\n" +
"  [\n" +
"    \"hello\",\n" +
"    \"world\"\n" +
"  ]\n" +
"  values:\n" +
"  [\n" +
"    1,\n" +
"    2\n" +
"  ]\n" +
"]">

diff:
+ [
+   keys:
+   [
+     "hello",
+     "world"
+   ]
? map<string, int8>[{key:string = hello, value :int8 = 1}, {key:string = world, value:int8 = 2}]
?                                             s
? ??????????????????????????????????????                                           + ------------------------------------------------
+   [
+     1,
+     2
+   ]
+ ]
==================================================================================================================================================================================================
F
==================================================================================================================================================================================================
Failure: test_to_s(TestMonthDayNanoIntervalScalar)
/Users/lama/workspace/arrow-latest/c_glib/test/test-month-day-nano-interval-scalar.rb:41:in `test_to_s'
     38:   end
     39:
     40:   def test_to_s
  => 41:     assert_equal("3M10d100ns", @scalar.to_s)
     42:   end
     43:
     44:   def test_value
<"3M10d100ns"> expected but was
<"[\n" + "  3M10d100ns\n" + "]">

diff:
+ [
?   3M10d100ns
+ ]
==================================================================================================================================================================================================
F
==================================================================================================================================================================================================
Failure: test_to_s(TestMonthIntervalScalar)
/Users/lama/workspace/arrow-latest/c_glib/test/test-month-interval-scalar.rb:41:in `test_to_s'
     38:   end
     39:
     40:   def test_to_s
  => 41:     assert_equal("1M", @scalar.to_s)
     42:   end
     43:
     44:   def test_value
<"1M"> expected but was
<"[\n" + "  1M\n" + "]">

diff:
+ [
?   1M
+ ]
==================================================================================================================================================================================================
F
==================================================================================================================================================================================================
Failure: test_to_s(TestSparseUnionScalar)
/Users/lama/workspace/arrow-latest/c_glib/test/test-sparse-union-scalar.rb:52:in `test_to_s'
     49:   end
     50:
     51:   def test_to_s
  => 52:     assert_equal("union{number: int8 = -29}", @scalar.to_s)
     53:   end
     54:
     55:   def test_value
<"union{number: int8 = -29}"> expected but was
<"-- is_valid: all not null\n" +
"-- type_ids:   [\n" +
"    2\n" +
"  ]\n" +
"-- child 0 type: int8\n" +
"  [\n" +
"    -29\n" +
"  ]\n" +
"-- child 1 type: string\n" +
"  [\n" +
"    null\n" +
"  ]">

diff:
+ -- is_valid: all not null
+ -- type_ids:   [
+     2
+   ]
? un   ion{numb er: int8 = -29}
? -- ch ld 0 typ
? ?? ??????? -      -------
+   [
+     -29
+   ]
+ -- child 1 type: string
+   [
+     null
+   ]
==================================================================================================================================================================================================
F
==================================================================================================================================================================================================
Failure: test_to_s(TestStructScalar)
/Users/lama/workspace/arrow-latest/c_glib/test/test-struct-scalar.rb:51:in `test_to_s'
     48:   end
     49:
     50:   def test_to_s
  => 51:     assert_equal("{score:int8 = -29, enabled:bool = true}", @scalar.to_s)
     52:   end
     53:
     54:   def test_value
<"{score:int8 = -29, enabled:bool = true}"> expected but was
<"-- is_valid: all not null\n" +
"-- child 0 type: int8\n" +
"  [\n" +
"    -29\n" +
"  ]\n" +
"-- child 1 type: bool\n" +
"  [\n" +
"    true\n" +
"  ]">

diff:
+ -- is_valid: all not null
? {s cor        e: int8 = -29, enabled:bool = true}
? --  hild 0 typ
? ?? ??  +    ----------------------------
+   [
+     -29
+   ]
+ -- child 1 type: bool
+   [
+     true
+   ]
==================================================================================================================================================================================================
\/Users/lama/workspace/arrow-latest/cpp/src/gandiva/cache.cc:50: Creating gandiva cache with capacity of 500
/Users/lama/workspace/arrow-latest/cpp/src/gandiva/engine.cc:265: Detected CPU Name : apple-m1
/Users/lama/workspace/arrow-latest/cpp/src/gandiva/engine.cc:266: Detected CPU Features: []
Finished in 72.076868 seconds.
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
1922 tests, 2092 assertions, 8 failures, 0 errors, 0 pendings, 14 omissions, 0 notifications
99.5807% passed
--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
26.67 tests/s, 29.02 assertions/s

llama90 avatar Feb 03 '24 09:02 llama90

Could you help me identify any broken workflows related to Python? I searched the codebase for CastTo, but couldn't find it in the Python parts. I'm puzzled as to why the workflow error mentions CastTo.

  • https://github.com/apache/arrow/actions/runs/7765982118/job/21181283847?pr=39192
    • pyarrow_cython_example.cpp:8782:46: error: 'using element_type = std::remove_extent<arrow::Scalar>::type' {aka 'struct arrow::Scalar'} has no member named 'CastTo'; did you mean 'cast'?

cc @AlenkaF @jorisvandenbossche

llama90 avatar Feb 03 '24 12:02 llama90

Could you help me identify any broken workflows related to Python? I searched the codebase for CastTo, but couldn't find it in the Python parts. I'm puzzled as to why the workflow error mentions CastTo.

  • https://github.com/apache/arrow/actions/runs/7765982118/job/21181283847?pr=39192

    • pyarrow_cython_example.cpp:8782:46: error: 'using element_type = std::remove_extent<arrow::Scalar>::type' {aka 'struct arrow::Scalar'} has no member named 'CastTo'; did you mean 'cast'?

There are occurrences in Cython that still bind to/use CastTo from C++ and so Cython tests fail, see https://github.com/search?q=repo%3Aapache%2Farrow+CastTo+language%3ACython&type=code&l=Cython

AlenkaF avatar Feb 03 '24 14:02 AlenkaF

We want to address issues that are connected with other languages (ruby, python..) first, especially those parts that can be solved independently for each language.

  • #40023

llama90 avatar Feb 10 '24 05:02 llama90