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: brits imputation test device mismatch #11

Merged
merged 3 commits into from
Aug 22, 2022

Conversation

MaciejSkrabski
Copy link
Contributor

attempt to fix #10

@MaciejSkrabski
Copy link
Contributor Author

By the way, may I suggest using code autoformatter named black? 😁

@WenjieDu
Copy link
Owner

Hi Maciej, thank you for your patience! I'm sorry for my delayed response. I have run the test cases and the results look fine. I only have one comment for your PR 😉 When you try to fix a bug, please only modify the code that is related to the bug because this can help the reviewer quickly understand what you are working on.

And BTW, what do you mean by 'using code autoformatter named black'? Sounds like a configuration of repository settings on GitHub.

Last but not least, many thanks for your PR! 😃

@MaciejSkrabski
Copy link
Contributor Author

Hi Wenjie, thank you for reviewing the PR!
Your insight is on point. I really shouldn't have tinkered with the code that does not concern the issue at hand, especially when it is just cosmetics, as it makes analysing code more difficult. I am sorry for being overzealous 😁

What I do mean about 'using code autoformatter named black': there's this thing called black. It is a python autoformatter, that obides the pep8 code style. It can be used as a standalone program run on *.py files. In that regard it is similar to python linters like pylint. As it can be used in a standalone manner, I guess it is possible to include it in your GitHub workflows. I think it would either create a new commit on top of what is being pushed to the branch, or ammend the last commit. It can also be used as part of your IDE / text editor while first writing the code - I know it is a no-brainer to setup with VSCode. My colleague, who uses Eclipse, had to tinker with it for a while.

Anyway, black makes the indents right, the quotes consistent, the function arguments neatly organised and so much more! It can be parametrised a bit, and the config is just a file that you can keep in the repo, so people automatically use the code style you ask them to, as long as they use black.. Its greatest advantage is that anyone working with the code shares the same style without ever giving it a single thought.

On the other hand, when one party does not use black, and the second party does, the thing that happened to the two of us happens - me, not realising how convoluted my PR will appear to a reviewer, had black configured to run on-file-save, so whenever I hit that CTRL+S shortcut, the python file gets formatted for me. This produces a lot of "file changed" messages on github, all of them just cosmetic.

I swear I am not being payed for advertising black 😛

I should not have interfered with good code, which does not concern the issue, just for cosmetics sake. I apologise.

@WenjieDu
Copy link
Owner

I see. Thank you very much for your explanation! Let's merge your code, Maciej👍

@WenjieDu WenjieDu merged commit 1f9c44f into WenjieDu:dev Aug 22, 2022
@WenjieDu
Copy link
Owner

Merged. Regarding your recommendation for using black, I will also give it consideration. Thanks for letting me know about this formatting tool. 😊

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.

None yet

2 participants