Use modern names for pyparsing methods #512
Conversation
MiWeiss
left a comment
MiWeiss
left a comment
There was a problem hiding this comment.
Thanks a lot! Just had a quick look, and just one comment.
| def parseFile(self, file_obj): | ||
| return self.main_expression.parseFile(file_obj, parseAll=True) | ||
| def parse_file(self, file_obj): | ||
| return self.main_expression.parse_file(file_obj, parse_all=True) |
There was a problem hiding this comment.
This looks like a breaking change, rather than just calling different pyparsing method.
Not sure if intendend, but if intended: I can see why that's done - essentially its just a wrapper around the pyparsing method with the same name. Still, to prevent the breaking change, I'd rather introduce an alias method, rather than replacing parseFile in our API.
There was a problem hiding this comment.
Good catch. I'm surprised the conversion tool did that but I can understand why. I will revert this line.
There was a problem hiding this comment.
I see that the method is called a few times in bparser.py. Since I'm not 100% whether it always has a BibtexExpression and not a pyparsing object when those calls are made, I added an alias rather than reverting the change.
There was a problem hiding this comment.
I think I addressed your comment. Just pinging in case this fell off your radar. :-)
2 participants
Names converted via
python -m pyparsing.tools.cvt_pyparsing_pep8_names -u *.py, adjusting the path to cover all python source files.Fixes #502