You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "CurtHagenlocher (via GitHub)" <gi...@apache.org> on 2023/05/22 20:07:35 UTC
[GitHub] [arrow] CurtHagenlocher opened a new pull request, #35716: GH-33032: [C#] Support fixed-size lists
CurtHagenlocher opened a new pull request, #35716:
URL: https://github.com/apache/arrow/pull/35716
### What changes are included in this PR?
Support fixed-size lists in the C# implementation.
Adds Archery support for Lists, Structs and Fixed-size Lists in the C# implementation.
### Are these changes tested?
Yes.
### Are there any user-facing changes?
Fixed-size lists are now supported for C#.
--
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] lidavidm commented on pull request #35716: GH-33032: [C#] Support fixed-size lists
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on PR #35716:
URL: https://github.com/apache/arrow/pull/35716#issuecomment-1686991700
Alright, looks like integration and docs failures are unrelated
--
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] CurtHagenlocher commented on a diff in pull request #35716: GH-33032: [C#] Support fixed-size lists
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher commented on code in PR #35716:
URL: https://github.com/apache/arrow/pull/35716#discussion_r1300535929
##########
csharp/src/Apache.Arrow/Types/FixedSizeListType.cs:
##########
@@ -0,0 +1,39 @@
+// 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.
+
+namespace Apache.Arrow.Types
+{
+ public sealed class FixedSizeListType : NestedType
+ {
+ public override ArrowTypeId TypeId => ArrowTypeId.FixedSizeList;
+ public override string Name => "fixed_size_list";
+ public int ListSize { get; }
+
+ public Field ValueField => Fields[0];
+
+ public IArrowType ValueDataType => Fields[0].DataType;
+
+ public FixedSizeListType(Field valueField, int listSize)
Review Comment:
Derp.... not sure what I did :(
--
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] lidavidm commented on a diff in pull request #35716: GH-33032: [C#] Support fixed-size lists
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm commented on code in PR #35716:
URL: https://github.com/apache/arrow/pull/35716#discussion_r1300257036
##########
csharp/src/Apache.Arrow/Types/FixedSizeListType.cs:
##########
@@ -0,0 +1,39 @@
+// 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.
+
+namespace Apache.Arrow.Types
+{
+ public sealed class FixedSizeListType : NestedType
+ {
+ public override ArrowTypeId TypeId => ArrowTypeId.FixedSizeList;
+ public override string Name => "fixed_size_list";
+ public int ListSize { get; }
+
+ public Field ValueField => Fields[0];
+
+ public IArrowType ValueDataType => Fields[0].DataType;
+
+ public FixedSizeListType(Field valueField, int listSize)
Review Comment:
Worth validating that listSize > 0?
--
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] github-actions[bot] commented on pull request #35716: GH-33032: [C#] Support fixed-size lists
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35716:
URL: https://github.com/apache/arrow/pull/35716#issuecomment-1557895952
:warning: GitHub issue #33032 **has been automatically assigned in GitHub** to PR creator.
--
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] conbench-apache-arrow[bot] commented on pull request #35716: GH-33032: [C#] Support fixed-size lists
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #35716:
URL: https://github.com/apache/arrow/pull/35716#issuecomment-1687847732
After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 37a9f27e658474a929f351c89c90159b401fb8ca.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/16099011220) has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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] CurtHagenlocher commented on a diff in pull request #35716: GH-33032: [C#] Support fixed-size lists
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher commented on code in PR #35716:
URL: https://github.com/apache/arrow/pull/35716#discussion_r1265807203
##########
csharp/src/Apache.Arrow/Field.cs:
##########
@@ -35,24 +35,24 @@ public partial class Field
public Field(string name, IArrowType dataType, bool nullable,
IEnumerable<KeyValuePair<string, string>> metadata = default)
- : this(name, dataType, nullable)
+ : this(name, dataType, nullable, false)
{
Metadata = metadata?.ToDictionary(kv => kv.Key, kv => kv.Value);
}
internal Field(string name, IArrowType dataType, bool nullable,
- IReadOnlyDictionary<string, string> metadata, bool copyCollections)
- : this(name, dataType, nullable)
+ IReadOnlyDictionary<string, string> metadata, bool copyCollections, bool allowBlankName)
+ : this(name, dataType, nullable, allowBlankName)
{
Debug.Assert(copyCollections == false, "This internal constructor is to not copy the collections.");
Metadata = metadata;
}
- private Field(string name, IArrowType dataType, bool nullable)
+ private Field(string name, IArrowType dataType, bool nullable, bool allowBlankName)
Review Comment:
Though perhaps this should be done more broadly as per #36588.
--
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] CurtHagenlocher commented on a diff in pull request #35716: GH-33032: [C#] Support fixed-size lists
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher commented on code in PR #35716:
URL: https://github.com/apache/arrow/pull/35716#discussion_r1300533684
##########
csharp/src/Apache.Arrow/Types/FixedSizeListType.cs:
##########
@@ -0,0 +1,39 @@
+// 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.
+
+namespace Apache.Arrow.Types
+{
+ public sealed class FixedSizeListType : NestedType
+ {
+ public override ArrowTypeId TypeId => ArrowTypeId.FixedSizeList;
+ public override string Name => "fixed_size_list";
+ public int ListSize { get; }
+
+ public Field ValueField => Fields[0];
+
+ public IArrowType ValueDataType => Fields[0].DataType;
+
+ public FixedSizeListType(Field valueField, int listSize)
Review Comment:
Added. (Also updated with latest source and fixed merge conflict.)
--
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] lidavidm merged pull request #35716: GH-33032: [C#] Support fixed-size lists
Posted by "lidavidm (via GitHub)" <gi...@apache.org>.
lidavidm merged PR #35716:
URL: https://github.com/apache/arrow/pull/35716
--
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] CurtHagenlocher commented on a diff in pull request #35716: GH-33032: [C#] Support fixed-size lists
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher commented on code in PR #35716:
URL: https://github.com/apache/arrow/pull/35716#discussion_r1300538728
##########
csharp/src/Apache.Arrow/Types/FixedSizeListType.cs:
##########
@@ -0,0 +1,39 @@
+// 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.
+
+namespace Apache.Arrow.Types
+{
+ public sealed class FixedSizeListType : NestedType
+ {
+ public override ArrowTypeId TypeId => ArrowTypeId.FixedSizeList;
+ public override string Name => "fixed_size_list";
+ public int ListSize { get; }
+
+ public Field ValueField => Fields[0];
+
+ public IArrowType ValueDataType => Fields[0].DataType;
+
+ public FixedSizeListType(Field valueField, int listSize)
Review Comment:
...ah, it was a conflict with a different change of mine that's been checked in. Fixed.
--
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] CurtHagenlocher commented on a diff in pull request #35716: GH-33032: [C#] Support fixed-size lists
Posted by "CurtHagenlocher (via GitHub)" <gi...@apache.org>.
CurtHagenlocher commented on code in PR #35716:
URL: https://github.com/apache/arrow/pull/35716#discussion_r1201005376
##########
csharp/src/Apache.Arrow/Field.cs:
##########
@@ -35,24 +35,24 @@ public partial class Field
public Field(string name, IArrowType dataType, bool nullable,
IEnumerable<KeyValuePair<string, string>> metadata = default)
- : this(name, dataType, nullable)
+ : this(name, dataType, nullable, false)
{
Metadata = metadata?.ToDictionary(kv => kv.Key, kv => kv.Value);
}
internal Field(string name, IArrowType dataType, bool nullable,
- IReadOnlyDictionary<string, string> metadata, bool copyCollections)
- : this(name, dataType, nullable)
+ IReadOnlyDictionary<string, string> metadata, bool copyCollections, bool allowBlankName)
+ : this(name, dataType, nullable, allowBlankName)
{
Debug.Assert(copyCollections == false, "This internal constructor is to not copy the collections.");
Metadata = metadata;
}
- private Field(string name, IArrowType dataType, bool nullable)
+ private Field(string name, IArrowType dataType, bool nullable, bool allowBlankName)
Review Comment:
I concede that this change is a bit odd; the intent was to address #32886.
--
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] github-actions[bot] commented on pull request #35716: GH-33032: [C#] Support fixed-size lists
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35716:
URL: https://github.com/apache/arrow/pull/35716#issuecomment-1557910184
:warning: GitHub issue #33032 **has been automatically assigned in GitHub** to PR creator.
--
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] github-actions[bot] commented on pull request #35716: GH-33032: [C#] Support fixed-size lists
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #35716:
URL: https://github.com/apache/arrow/pull/35716#issuecomment-1557895901
* Closes: #33032
--
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