[GitHub] [arrow] JaapSuter opened a new pull request, #14677: [JS] Add minor caution to Vector.toArray doc-comment about how it handles nulls (it doesn't).

[GitBox <gi...@apache.org>]

JaapSuter opened a new pull request, #14677:
URL: https://github.com/apache/arrow/pull/14677

   This just adds a minor cautionary note to Vector.toArray, the diff is self-explanatory.


-- 
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] amol- commented on pull request #14677: MINOR: [JS] Add caution to Vector.toArray doc-comment about how it handles nulls (it doesn't).

["amol- (via GitHub)" <gi...@apache.org>]

amol- commented on PR #14677:
URL: https://github.com/apache/arrow/pull/14677#issuecomment-1490646034

   Closing because it has been untouched for a while, in case it's still relevant feel free to reopen and move it forward 👍


-- 
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 #14677: [JS] Add minor caution to Vector.toArray doc-comment about how it handles nulls (it doesn't).

[GitBox <gi...@apache.org>]

github-actions[bot] commented on PR #14677:
URL: https://github.com/apache/arrow/pull/14677#issuecomment-1320409624

   <!--
     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.
   -->
   
   Thanks for opening a pull request!
   
   If this is not a [minor PR](https://github.com/apache/arrow/blob/master/CONTRIBUTING.md#Minor-Fixes). Could you open an issue for this pull request on JIRA? https://issues.apache.org/jira/browse/ARROW
   
   Opening JIRAs ahead of time contributes to the [Openness](http://theapacheway.com/open/#:~:text=Openness%20allows%20new%20users%20the,must%20happen%20in%20the%20open.) of the Apache Arrow project.
   
   Then could you also rename pull request title in the following format?
   
       ARROW-${JIRA_ID}: [${COMPONENT}] ${SUMMARY}
   
   or
   
       MINOR: [${COMPONENT}] ${SUMMARY}
   
   See also:
   
     * [Other pull requests](https://github.com/apache/arrow/pulls/)
     * [Contribution Guidelines - How to contribute patches](https://arrow.apache.org/docs/developers/contributing.html#how-to-contribute-patches)
   


-- 
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] domoritz commented on a diff in pull request #14677: MINOR: [JS] Add caution to Vector.toArray doc-comment about how it handles nulls (it doesn't).

[GitBox <gi...@apache.org>]

domoritz commented on code in PR #14677:
URL: https://github.com/apache/arrow/pull/14677#discussion_r1026962954


##########
js/src/vector.ts:
##########
@@ -241,6 +241,11 @@ export class Vector<T extends DataType = any> {
      *  more than one chunk, the resulting TypedArray will be a copy of the data from each
      *  chunk's underlying TypedArray values.
      *
+     * @note This does _not_ respect null values. After all, if a typed array is returned
+     *  it can't contain nulls anyway. So be aware that calling toArray is not the same
+     *  as creating your own array copy using Vector.get(index) in a for-loop. In other
+     *  words, this function may not do what you expect when Vector.nullCount isn't zero.
+     *

Review Comment:
   I think this note can be more concise and mention toJSON as an alternative to get nulls (albeit it's less performant). 



-- 
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] amol- closed pull request #14677: MINOR: [JS] Add caution to Vector.toArray doc-comment about how it handles nulls (it doesn't).

["amol- (via GitHub)" <gi...@apache.org>]

amol- closed pull request #14677: MINOR: [JS] Add caution to Vector.toArray doc-comment about how it handles nulls (it doesn't).
URL: https://github.com/apache/arrow/pull/14677


-- 
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