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

Fix misaligned arguments in super() constructor call #6

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

bauwenst
Copy link

@bauwenst bauwenst commented Dec 9, 2024

Fixes #5 by adding keyword arguments to the constructor. Also did some minor refactoring.

@zipzou
Copy link
Owner

zipzou commented Dec 15, 2024

Good job!! But I think it is better that all the codes are reformatted with yapf according to the .style.cfg. This could reduce the diff and keep the style consistent with before.

Well, I would pull these changes, reformat and review them later. If you have more free time to reformat codes, just make a new submission or PR.

Thanks for your time!

@bauwenst
Copy link
Author

bauwenst commented Dec 15, 2024

I think it is better that all the codes are reformatted with yapf according to the .style.cfg. This could reduce the diff and keep the style consistent with before.

I see. I don't use this reformatting tool, and instead I try to manually adhere to the usual style used by the HuggingFace team. I have to admit that some of the formatting in your code is quite strange to me (e.g. using three lines for simple function calls), to the point that I've never seen anyone style Python like it. It reminds me of Java, but Python is not Java ;-)

Up to you how you want to resolve this. I'm obviously not going to re-format code in a style I don't know :P

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.

super().__init__() call should use keyword arguments
2 participants