Skip to content
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

[QUESTION] Why syntax errors are raised as IOError and not SyntaxError? #74

Open
izimobil opened this issue May 13, 2016 · 8 comments
Open

Comments

@izimobil
Copy link
Owner

Originally reported by: Luis Alejandro Martínez Faneyth (Bitbucket: LuisAlejandro, GitHub: LuisAlejandro)


I've noticed that syntax errors are being raised as IOError, and i would like if there's a special reason for this.

This affects me because I can't access the line number of the syntax error because IOError does not have that attribute.

Specifically, here:

https://bitbucket.org/izi/polib/src/ac206f2b25956f95155fb1dcbc881068e3a15bfe/polib.py?at=default&fileviewer=file-view-default#polib.py-1382

I can't access current_line either in a try-except. And parsing the exception message with regex is not the best solution for me.

Thanks for your time.


@izimobil
Copy link
Owner Author

izimobil commented May 13, 2016

Original comment by David Jean Louis (Bitbucket: izi, GitHub: izimobil):


There's no particular reason... it's just a bad exception choice that I made very long ago (10 years !).

The problem is that a lot of codebase and scripts are relying on this initial bad choice, fixing this behavior will break backwards compatibility, so unfortunately I can't change this :(

@izimobil
Copy link
Owner Author

Original comment by Luis Alejandro Martínez Faneyth (Bitbucket: LuisAlejandro, GitHub: LuisAlejandro):


Perhaps you could add the attributes to the IOError instance like:

IOError.filename = self.instance.fpath
IOError.linenum = self.current_line

I dont know if "filename" and "linenum" are the names of the actual attributes of SyntaxError, you'll have to look out.

Or, perhaps you could make a custom exception for your library which subclasses IOError (POException?) and that way you can add these attributes and not brake others codebases. Subclassing IOError will trigger their exceptions the same way.

I'm willing to help if you decide to go for it.

Thanks.

@izimobil
Copy link
Owner Author

izimobil commented May 13, 2016

Original comment by David Jean Louis (Bitbucket: izi, GitHub: izimobil):


Yeah, we could subclass IOError in a POFileError class and add the relevant attributes (I strongly prefer this over monkeypatching IOError). If you're willing to help, please go ahead and submit a pull request with preferably tests where possible. Thanks in advance.

@izimobil
Copy link
Owner Author

Original comment by Luis Alejandro Martínez Faneyth (Bitbucket: LuisAlejandro, GitHub: LuisAlejandro):


Excellent, i'll be reporting as soon as i can.

@izimobil
Copy link
Owner Author

izimobil commented Nov 27, 2017

Original comment by David Jean Louis (Bitbucket: izi, GitHub: izimobil):


@LuisAlejandro I know it's been a long time, but do you still plan to send a pull request for this ? Thanks.

@izimobil izimobil added trivial and removed TIP labels Nov 27, 2017
@MestreLion
Copy link
Contributor

Original comment by David Jean Louis (Bitbucket: izi, GitHub: izimobil):

Yeah, we could subclass IOError in a POFileError class and add the relevant attributes (I strongly prefer this over monkeypatching IOError). If you're willing to help, please go ahead and submit a pull request with preferably tests where possible. Thanks in advance.

A simpler and perhaps better solution is for POFileError to simply subclass from both IOError and SyntaxError. it would automatically have all the needed attributes from SyntaxError and still trigger any legacy code that expects an IOError:

class POFileSyntaxError(IOError, SyntaxError): pass

There, the new class is ready to be used throughout the code, with all the attributes from both its superclasses. Python's multiple inheritance is amazing!

@izimobil
Copy link
Owner Author

@MestreLion : no, you can't do that, it would result in conflicts:

Python 3.9.1 (default, Dec  8 2020, 03:26:34) 
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> class Foo(SyntaxError, IOError):
...     pass
... 
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: multiple bases have instance lay-out conflict

@MestreLion
Copy link
Contributor

Oh, bummer. I have used this strategy before, IIRC mixing KeyError and IndexError (it works) . I guess SyntaxError cannot be mixed with others (IOError plays nicely with other errors)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants