rustyconover commented on code in PR #424:
URL: https://github.com/apache/arrow-dotnet/pull/424#discussion_r3846752944
##########
src/Apache.Arrow/Ipc/ArrowStreamWriter.cs:
##########
@@ -882,6 +910,7 @@ private protected async Task
WriteRecordBatchInternalAsync(RecordBatch recordBat
long metadataLength = await
WriteMessageAsync(Flatbuf.MessageHeader.RecordBatch,
recordBatchOffset, recordBatchBuilder.TotalLength,
+ customMetadataVectorOffset,
cancellationToken).ConfigureAwait(false);
Review Comment:
Fixed in 0177e8b: `FlightDataStream` now overrides the customMetadata-aware
`WriteMessageAsync` overload (the one `WriteRecordBatchInternalAsync` actually
calls) instead of the old 4-arg one, so Flight record batch writes are no
longer silently routed through the base implementation.
##########
src/Apache.Arrow/Ipc/ArrowStreamWriter.cs:
##########
@@ -829,6 +834,14 @@ private protected void
WriteRecordBatchInternal(RecordBatch recordBatch)
VectorOffset buffersVectorOffset = Builder.EndVector();
+ // Build custom metadata for the Message if provided
+ VectorOffset customMetadataVectorOffset = default;
+ if (customMetadata != null && customMetadata.Count > 0)
+ {
+ Offset<Flatbuf.KeyValue>[] metadataOffsets =
GetMetadataOffsets(customMetadata);
+ customMetadataVectorOffset =
Flatbuf.Message.CreateCustomMetadataVector(Builder, metadataOffsets);
Review Comment:
Fixed in 0177e8b: added a `ValidateCustomMetadata` check that throws a clear
`ArgumentException` for null keys/values before building the FlatBuffer offsets.
##########
src/Apache.Arrow/Ipc/ArrowStreamReader.cs:
##########
@@ -151,5 +152,12 @@ public RecordBatch ReadNextRecordBatch()
{
return _implementation.ReadNextRecordBatch();
}
+
+ /// <summary>
+ /// Custom metadata from the most recently read RecordBatch Message.
+ /// Updated after each call to
ReadNextRecordBatch/ReadNextRecordBatchAsync.
+ /// Returns null if the last batch had no custom metadata.
+ /// </summary>
Review Comment:
Fixed in 0177e8b: reworded the doc comment to describe the actual semantics
— the property is left unchanged (not cleared) when a read call returns null,
e.g. at end of stream.
##########
test/Apache.Arrow.Tests/CustomMetadataPythonTests.cs:
##########
@@ -0,0 +1,183 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using Apache.Arrow.Ipc;
+using Python.Runtime;
+using Xunit;
+
+namespace Apache.Arrow.Tests
+{
+
+ // -------------------------------------------------------------------
+ // Cross-language Python tests for custom_metadata
+ // -------------------------------------------------------------------
+
+ public class CustomMetadataPythonTests :
IClassFixture<CustomMetadataPythonTests.PythonNet>
+ {
+ public class PythonNet : IDisposable
Review Comment:
Fixed in 0177e8b: the test class now uses the shared `PythonNetFixture` +
`[Collection("PythonNet")]` (same pattern as `CDataSchemaPythonTest`) instead
of its own private Python.NET init/shutdown.
##########
test/Apache.Arrow.Tests/CustomMetadataPythonTests.cs:
##########
@@ -0,0 +1,183 @@
+// Licensed to the Apache Software Foundation (ASF) under one or more
+// contributor license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright ownership.
+// The ASF licenses this file to You under the Apache License, Version 2.0
+// (the "License"); you may not use this file except in compliance with
+// the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using Apache.Arrow.Ipc;
+using Python.Runtime;
+using Xunit;
+
+namespace Apache.Arrow.Tests
+{
+
+ // -------------------------------------------------------------------
+ // Cross-language Python tests for custom_metadata
+ // -------------------------------------------------------------------
+
+ public class CustomMetadataPythonTests :
IClassFixture<CustomMetadataPythonTests.PythonNet>
+ {
+ public class PythonNet : IDisposable
+ {
+ public bool Initialized { get; }
+
+ public bool VersionMismatch { get; }
+
+ public PythonNet()
+ {
+ bool pythonSet =
Environment.GetEnvironmentVariable("PYTHONNET_PYDLL") != null;
+ if (!pythonSet)
+ {
+ Initialized = false;
+ return;
+ }
+
+ try
+ {
+ PythonEngine.Initialize();
+ }
+ catch (NotSupportedException e) when
(e.Message.Contains("Python ABI ") && e.Message.Contains("not supported"))
+ {
+ Initialized = false;
+ VersionMismatch = true;
+ return;
+ }
+
+ if
(System.Runtime.InteropServices.RuntimeInformation.IsOSPlatform(System.Runtime.InteropServices.OSPlatform.Windows)
&&
+ PythonEngine.PythonPath.IndexOf("dlls",
StringComparison.OrdinalIgnoreCase) < 0)
+ {
+ dynamic sys = Py.Import("sys");
+
sys.path.append(Path.Combine(Path.GetDirectoryName(Environment.GetEnvironmentVariable("PYTHONNET_PYDLL")),
"DLLs"));
+ }
Review Comment:
Fixed in 0177e8b as part of switching to the shared `PythonNetFixture`,
which already wraps the Windows `sys.path` append in `using (Py.GIL())`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
