-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added file closing functionality and added another unicode validation #14
base: master
Are you sure you want to change the base?
Conversation
I did little testing, it seems to work. But when you initialize it as before with g = Gbx(file) it's not getting closed anymore, since I removed the close function when raising the error. Or should we disable the usual initialization? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more diff comment.
Re closing: this is fine, if you init the object yourself, you're expected to close it manually, the docs will need to be updated. So the preferred way would be using the with
statement from now on.
pygbx/gbx.py
Outdated
@@ -80,6 +80,11 @@ def __init__(self, obj): | |||
self.root_parser = ByteReader(obj) | |||
|
|||
self.magic = self.root_parser.read(3, '3s') | |||
try: | |||
self.magic.decode('utf-8') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because magic is used only temporarily in the constructor, drop self
and reuse it:
magic = self.root_parser.read(3, '3s')
try:
magic = magic.decode('utf-8')
except:
raise GbxLoadError(f'obj is not a valid Gbx data: can not decode unicode')
if magic != 'GBX':
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good one. Pushed another version, which includes this change.
Re closing: The problem when initializing the object manually, and an GBXLoadError is raised, you are not able to close the connection manually, because the object is not completely initialized (g is None) but the file connection is still ongoing. You could close the connection on the Error object, but I don't think that is something which is recommended right? |
In case an error is raised when creating a GBX object the file connection is not being closed which is required, if you want to continue working on the file (e.g. deleting it).
Also when I tried to import an exe file, it failed raising the GbxLoadError, because it could not decode as utf-8. That's why I added another try block to potentially raise an GbxLoadError.
I removed all the format changes and I am looking into context functionality.