API server code health improvements #1041
Conversation
There was a problem hiding this comment.
oof, some of the code here was really gnarly (especially the first file, src/acquisition/afhsb/afhsb_csv.py)! i looooove seeing all the unused imports and variables getting removed! the whitespace fixes also soothe my ocd. this is fabulous!!!
what programs/packages/commands/parameters did you use to do this? its probably something that should be incorporated into #962...
you should change "Closes #998" to "Addresses #998" and we can get this PR merged by itself. itll be good to separate this set of problematic code changes that a linter found automatically (syntactic points) from the other semantic or logical changes that youll make later in further pursuit of issue #998.
| def handle_failed(_, filename, source, logger): | ||
| logger.info(event='leaving failed file alone', dest=source, file=filename) | ||
|
|
||
| def handle_successful(path_src, filename, source, logger): | ||
| def handle_successful(path_src, filename, _, logger): |
There was a problem hiding this comment.
i think we should leave these as-is because they are intended to have the same function signature.
| region = NIDSS.LOCATION_TO_REGION[location] | ||
| imported_b64 = base64.b64encode(fields[6].encode('utf-8')) | ||
| imported = imported_b64 == b'5piv' | ||
| sex = fields[5] | ||
| age = fields[7] |
There was a problem hiding this comment.
maybe just comment out the region, sex, and age lines and note that theyre currently unused? leaving them in could help someone debug later (at least in terms of parsing the input file), and they might even be data points we will add in the future...
Co-authored-by: melange396 <george.haff@gmail.com>
|
@melange396 I applied the changes you've requested!
This PR fixes issues detected by Prospector, which uses pylint internally. There are tools that can autoformat Python repos, which we can use now that we have a working editable package. black is one that comes to mind: it can be incorporated as a CI step (yells at you if your code isn't formatted), post-commit hook (formats your code automatically), etc. Caveat: these tools are non-configurable and "uncompromising" by design, so on an initial run, they will reformat the majority of the codebase to the indentation style they like, affecting most lines of code. This will
so we need to think carefully about when to apply them. |
rzats
changed the title
Addresses #998.
Prerequisites:
devbranchdevSummary
General linting pass for the API & acquisition code: removes unused imports, fixes some whitespace issues, fixing exception try/except blocks etc