Skip to content

Commit

Permalink
package: Seperate imports of sys package from others (#260)
Browse files Browse the repository at this point in the history 
Signed-off-by: Peace Lee <iipeace5@gmail.com>
  • Loading branch information
@iipeace
iipeace committed Sep 25, 2021
1 parent f9fee33 commit 22a623e
Showing 1 changed file with 8 additions and 2 deletions.
10 changes: 8 additions & 2 deletions guider/guider.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,18 @@



# import sys package #
try:
import sys
except ImportError:
print("[ERROR] fail to import sys package")
sys.exit(0)

This comment has been minimized.

Sign in to view

Copy link
@elfring

elfring Sep 28, 2021

This statement is still questionable here. 🤔
How do you think about to omit additional exception handling for such a special case?

This comment has been minimized.

Sign in to view

Copy link
@iipeace

iipeace Sep 29, 2021

Author Owner

I think It looks simpler!
then how about this code?

# import essential packages #
try:
    import sys
    import os
    import re
    import gc
    import time
    import errno
    import signal
    import atexit
    import struct
    from copy import deepcopy
    #from ctypes import *
except ImportError:
    err = sys.exc_info()[1]
    print("[ERROR] failed to import essential package: %s" % err.args[0])
    sys.exit(0)

This comment has been minimized.

Sign in to view

Copy link
@elfring

elfring Sep 29, 2021

I propose to move the statement “import sys” before the “try”.

This comment has been minimized.

Sign in to view

Copy link
@iipeace

iipeace Sep 29, 2021

Author Owner

Ok!
I got it :)

# import essential packages #
try:
import os
import re
import gc
import sys
import time
import errno
import signal
Expand All @@ -31,7 +37,7 @@
#from ctypes import *
except ImportError:
err = sys.exc_info()[1]
print("[ERROR] fail to import essential packages: %s" % err.args[0])
print("[ERROR] fail to import essential package: %s" % err.args[0])
sys.exit(0)

This comment has been minimized.

Sign in to view

Copy link
@elfring

elfring Sep 28, 2021

I propose to pass the error message to the exit() method call.

This comment has been minimized.

Sign in to view

Copy link
@iipeace

iipeace Sep 29, 2021

Author Owner

That's a good idea!

# convert an unsupported type #
Expand Down

2 comments on commit 22a623e

@elfring
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • It would have been nice to avoid a typo in the commit subject.
  • I would appreciate more helpful commit descriptions.

@iipeace
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your feedback!

Please sign in to comment.