You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2020/06/12 10:17:45 UTC

[GitHub] [arrow] cool-RR opened a new pull request #7416: Fix exception causes in cli.py

cool-RR opened a new pull request #7416:
URL: https://github.com/apache/arrow/pull/7416


   I recently went over [Matplotlib](https://github.com/matplotlib/matplotlib/pull/16706), [Pandas](https://github.com/pandas-dev/pandas/pull/32322) and [NumPy](https://github.com/numpy/numpy/pull/15731), fixing a small mistake in the way that Python 3's exception chaining is used. If you're interested, I can do it here too. I've done it on just one file right now. 
   
   The mistake is this: In some parts of the code, an exception is being caught and replaced with a more user-friendly error. In these cases the syntax `raise new_error from old_error` needs to be used.
   
   Python 3's exception chaining means it shows not only the traceback of the current exception, but that of the original exception (and possibly more.) This is regardless of `raise from`. The usage of `raise from` tells Python to put a more accurate message between the tracebacks. Instead of this: 
   
       During handling of the above exception, another exception occurred:
   
   You'll get this: 
   
       The above exception was the direct cause of the following exception:
   
   The first is inaccurate, because it signifies a bug in the exception-handling code itself, which is a separate situation than wrapping an exception.
   
   Let me know what you think! 


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

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



[GitHub] [arrow] wesm commented on pull request #7416: ARROW-9113: [Archery] Fix exception causes in cli.py

Posted by GitBox <gi...@apache.org>.
wesm commented on pull request #7416:
URL: https://github.com/apache/arrow/pull/7416#issuecomment-643236620


   To be honest I wasn't even aware of this feature, since for the longest time I've been developing 2.7/3.x hybrid codebases where syntactic features like this are off limits. In principle I'm in favor of using this everywhere, interested what @pitrou or others think


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

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



[GitHub] [arrow] cool-RR closed pull request #7416: ARROW-9113: [Archery] Fix exception causes in cli.py

Posted by GitBox <gi...@apache.org>.
cool-RR closed pull request #7416:
URL: https://github.com/apache/arrow/pull/7416


   


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

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



[GitHub] [arrow] pitrou commented on pull request #7416: ARROW-9113: [Archery] Fix exception causes in cli.py

Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #7416:
URL: https://github.com/apache/arrow/pull/7416#issuecomment-643378995


   To be honest, I think those are all fine. There isn't much to be gained by forcing ourselves to spell out `raise from ...` explicitly, and we'll inevitably overlook some possible instances.


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

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



[GitHub] [arrow] github-actions[bot] commented on pull request #7416: ARROW-9113: [Archery] Fix exception causes in cli.py

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7416:
URL: https://github.com/apache/arrow/pull/7416#issuecomment-643206492


   https://issues.apache.org/jira/browse/ARROW-9113


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

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



[GitHub] [arrow] cool-RR commented on pull request #7416: ARROW-9113: [Archery] Fix exception causes in cli.py

Posted by GitBox <gi...@apache.org>.
cool-RR commented on pull request #7416:
URL: https://github.com/apache/arrow/pull/7416#issuecomment-643355049


   I understand. 
   
   I personally hate `from None`, and I think that when people are debugging they have a very different mindset than when they are writing code. When you're debugging, and you get a very limited amount of information that may have been copy-pasted in an email by a non-technical person, you're very desperate for the information.
   
   In this case, I would have wanted to see the `ArrowSources.find(src)` traceback. I would maybe get a hint on what happened. If it's a dead end for exploration, I at least want to know its content before I rule it out as a dead end.
   
   Another issue is that some error-reporting systems show not only the traceback, but all the local variables on each of these levels. These supposedly irrelevant levels could have variables that make it clearer what this exception is about.
   
   Below is a list of lines in arrow where you might want to change to either a `raise new from old` line or a `raise new from None` line. If you'd like me to change to a `raise new from old` on any of these, let me know and I'll create a PR:
   
   ```
   dev/archery/archery/bot.py: 57
   dev/archery/archery/cli.py: 94
   dev/archery/archery/cli.py: 768
   dev/archery/archery/cli.py: 773
   dev/archery/archery/docker.py: 131
   dev/archery/archery/integration/util.py: 148
   dev/archery/archery/lang/python.py: 180
   dev/merge_arrow_pr.py: 91
   dev/merge_arrow_pr.py: 540
   dev/tasks/crossbow.py: 351
   dev/tasks/crossbow.py: 651
   dev/tasks/crossbow.py: 817
   dev/tasks/crossbow.py: 1137
   python/pyarrow/pandas_compat.py: 416
   python/pyarrow/pandas_compat.py: 420
   python/pyarrow/pandas_compat.py: 426
   python/pyarrow/pandas_compat.py: 566
   python/pyarrow/tests/test_flight.py: 77
   python/pyarrow/tests/test_hdfs.py: 46
   python/pyarrow/tests/test_hdfs.py: 401
   run-cmake-format.py: 90
   ```


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

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



[GitHub] [arrow] github-actions[bot] commented on pull request #7416: Fix exception causes in cli.py

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7416:
URL: https://github.com/apache/arrow/pull/7416#issuecomment-643200828


   <!--
     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!
   
   Could you open an issue for this pull request on JIRA?
   https://issues.apache.org/jira/browse/ARROW
   
   Then could you also rename pull request title in the following format?
   
       ARROW-${JIRA_ID}: [${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.

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