Closed elsiehupp closed 11 months ago
Note also that this shouldn't have changed any actual functionality. It just should have made the Python more compliant.
Well, it looks like there's more for me to do! (And feel free to pitch in if the import errors look like something you can fix.)
For now I'm going to zone out and get ready for bed!
It looks like the main thing is I wasn't using Flake8, and the tests were expecting me to, so I guess I have to reconcile the linting configuration with that.
Should there be a mention of PEP 8 and Flake8 in contributing.md?
Should there be a mention of PEP 8 and Flake8 in contributing.md?
Probably yes, though they haven't been part of the development process so far, so we should add them incrementally.
I think this would be less overwhelming if I took a much more narrowly constrained approach, so I'm gonna close this and try again from scratch.
(The PEP-8 styling in particular really messed things up by creating a bunch of name collisions.)
If you're curious about mypy
, it's a development tool that makes Python much, much pickier, like Java. It doesn't actually run when you use the program; rather, it's a thing you use to preempt certain bugs, hence why the application works regardless already.
Should there be a mention of PEP 8 and Flake8 in contributing.md?
and mypy
I got
mypy
up and running on my computer, and hoo, boy. Anyway that was a big part of what I had been trying to do (and got bogged down doing) like two years ago.I also changed all the functions to use PEP 8-style snake-case function names, but I probably should have waited on that.
Note that I didn't 100% placate
mypy
, but I got like 99% of the way there.A bunch of module imports became substantially less elegant with all this. This inelegance shouldn't be too difficult to fix, but they should have been correct to begin with!
I'm not sure if this actually runs, because the tests don't run locally, and apparently
dumpgenerator
has been running just fine despite all sorts of egregious type errors. Yay Python!So I guess (a) please test this with your own workflows (in addition to the CI tests) and (b) maybe approve this if it passes both (despite being not 100% there)?
This not being 100% there is probably okay (if it works) since it's substantially more there than it was before...