Skip to content

Conversation

@jennifer-richards
Copy link

Names converted via python -m pyparsing.tools.cvt_pyparsing_pep8_names -u *.py, adjusting the path to cover all python source files.

Fixes #502

Copy link
Collaborator

@MiWeiss MiWeiss left a comment

Choose a reason for hiding this comment

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

Thanks a lot! Just had a quick look, and just one comment.

Comment on lines 277 to 278
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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Author

Choose a reason for hiding this comment

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

Good catch. I'm surprised the conversion tool did that but I can understand why. I will revert this line.

Copy link
Author

Choose a reason for hiding this comment

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

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants