Format update - Githubissues

mdolab / idwarp

IDWarp is a mesh warping package for the MACH framework.

Other

17 stars 29 forks source link

Format update #35

Closed marcomangano closed 3 years ago

marcomangano commented 3 years ago

Purpose

I re-formatted the code, as per this issue. The first two commits are the black and manual flake8 fixes respectively. I committed separately a few fixes that I would like the reviewers to double check carefully, to be sure that the code behaviour is not changing

Type of change

[ ] Bugfix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (non-backwards-compatible fix or feature)
[x] Code style update (formatting, renaming)
[ ] Refactoring (no functional changes, no API changes)
[ ] Documentation update
[x] Maintenance update
[ ] Other (please describe)

Checklist

[x] I have run flake8 and black to make sure the code adheres to PEP-8 and is consistently formatted
[ ] I have run unit and regression tests which pass locally with my changes
[ ] I have added new tests that prove my fix is effective or that my feature works
[ ] I have added necessary documentation

ewu63 commented 3 years ago

So I am a tiny bit uneasy about fixing complexify.py just because it's so old it needs a total overhaul. What I would recommend is reverting those files to how they were, and if you add a .flake8 file just to ignore those files, then Travis will work its magic. We can fix these at a later date.

See for example this file.

marcomangano commented 3 years ago

So I am a tiny bit uneasy about fixing complexify.py just because it's so old it needs a total overhaul. What I would recommend is reverting those files to how they were, and if you add a .flake8 file just to ignore those files, then Travis will work its magic. We can fix these at a later date.

See for example this file.

Yeah, I was a bit uneasy too but I gave it a shot. I will revert it back as you suggest

marcomangano commented 3 years ago

I will fix complexify by the end of the day

ewu63 commented 3 years ago

Sure, instead of reverting commits you can just copy/paste what we had before and do a single commit that way.

marcomangano commented 3 years ago

Sure, instead of reverting commits you can just copy/paste what we had before and do a single commit that way.

That's the only way I am able to do it ahaha

marcomangano commented 3 years ago

@nwu63 this should fix it?

ewu63 commented 3 years ago

Good except you still need to run black on it

marcomangano commented 3 years ago

Few minor things, otherwise looks good. Do we want to add the black/flake8 commit to a git blame ignore file?

@nwu63 what is our final position on this? I have one I can add now, but we realized that such file is only useful locally.. other ppl must initialize it in their git option for it to work, and still it is not effective on the browser interface. Your call guys!

ewu63 commented 3 years ago

Yeah personally I don't see much value in it since git cannot pick it out automatically, and it just sort of clutters the repo. Most tools can git blame multiple times and as long as your commit message is clear I think it's fine.