krober10nd / SeismicMesh

2D/3D serial and parallel triangular mesh generation tool for finite element methods.
https://seismicmesh.readthedocs.io/
GNU General Public License v3.0
127 stars 33 forks source link

pep8 compliance #51

Closed krober10nd closed 4 years ago

krober10nd commented 4 years ago

Ensuring compliance with pep8 per #48

1) All functions, variables, and methods obey snake_case.

2) All classes obey CapsCase.

nschloe commented 4 years ago

test_README failed. Ha! :+1:

krober10nd commented 4 years ago

haha, yes I'm a full believer in tests now. 👍 💯 Changing the api breaks everything and it helps enormously.

lgtm-com[bot] commented 4 years ago

This pull request introduces 3 alerts and fixes 1 when merging 76bf171369db78225323e30f06521672b47f7b46 into d0785545decd861cd2c499811299c881fc17daa8 - view on LGTM.com

new alerts:

fixed alerts:

nschloe commented 4 years ago

The changes in PR looks very good. One could even go a step further and look at the module name SeismicMesh. I for one am not aware of any Python package that uses capital letters. For package names, I usually stick to all small letters with no underscores/hyphens. This makes it possible to tell the user to pip install foobar and import foobar. Since this is a more intrusive call and not strictly necessary, I'll leave it up to you. In any case, something to keep in mind for the future.

krober10nd commented 4 years ago

looks like LGTM is helping also find what things broke in the API as it changes. Neat.

nschloe commented 4 years ago

looks like LGTM is helping also find what things broke in the API as it changes. Neat.

Yeah, didn't know that either.

krober10nd commented 4 years ago

The changes in PR looks very good.

Awesome, thank you. I'm finishing up some of the remaining modules and will post shortly. Then I'll tackle the more complex changes that need to happen to the public API that you suggested in the other issue.

One could even go a step further and look at the module name SeismicMesh. I for one am not aware of any Python package that uses capital letters. For package names, I usually stick to all small letters with no underscores/hyphens. This makes it possible to tell the user to pip install foobar and import foobar. Since this is a more intrusive call and not strictly necessary, I'll leave it up to you. In any case, something to keep in mind for the future.

Interesting! I clearly didn't realize this. Well, I don't know how that would work with the submitted manuscript and some people here using the programs. So for now, I'd rather leave it as is since it might confuse a bit of people. Definitely for my future developments I'll keep it mind and adhere to the standards.

lgtm-com[bot] commented 4 years ago

This pull request introduces 2 alerts and fixes 3 when merging 61c6e576150ca022fe85bcb8900859c6161245dc into d0785545decd861cd2c499811299c881fc17daa8 - view on LGTM.com

new alerts:

fixed alerts:

lgtm-com[bot] commented 4 years ago

This pull request fixes 3 alerts when merging 98aa86f26b7ccbcbfb2f4cb13769cdc55f1901f4 into d0785545decd861cd2c499811299c881fc17daa8 - view on LGTM.com

fixed alerts: