search

matplotlib / basemap

Plot on map projections (with coastlines and political boundaries) using matplotlib
MIT License
779 stars 392 forks source link

Enable flake8 #528

Open molinav opened 2 years ago

molinav commented 2 years ago

In the context of fixing current issues within basemap, one easy step is to use static analysis tools, since they can catch the most evident problems.

I open this issue as a reminder to enable flake8 in the development workflow; flake8 will catch the most evident code mistakes, and it will also give advice on changes that will make the code more pep8-compliant. Enabling pylint at this moment is not recommended, because it will complain too much.

molinav commented 2 years ago

Ideally we should run flake8 also in the examples. I remember that once I tried to run all of them and some were triggering errors (I cannot remember more details).

WeatherGod commented 2 years ago

I considered this about 5 years ago. Unfortunately, the codebase is very much not PEP8-compliant (I wouldn't be surprised if most of it predates a lot of PEP8). At the time, my goal was to shut down this project, so I didn't see any value in cleaning up the codebase. But, since your goals are different from mine, it might make sense to do some cleanup. I should note that the test suite is extremely sparse, and so I would stay away from auto-formatting tools like black that could make inadvertent, subtle changes that aren't covered by unit tests.

If anything, I'd suggest starting with converting the manually-run tests and turning them into automated image tests.

On Wed, Dec 29, 2021 at 9:55 AM Víctor Molina García < @.***> wrote:

Ideally we should run flake8 also in the examples. I remember that once I tried to run all of them and some were triggering errors (I cannot remember more details).

— Reply to this email directly, view it on GitHub https://github.com/matplotlib/basemap/issues/528#issuecomment-1002633499, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACHF6BP7DSRSM4IH6LB5W3UTMONJANCNFSM5K6C27EA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>