add pre-commit with ruff and typos, run --all-files #13
Conversation
| @@ -689,7 +745,7 @@ def terminology_entry_matches(terminology1, terminology2): | |||
| def segment_id_from_name(segmentation, segment_name): | |||
| segments = segmentation["segments"] | |||
| for segment in segments: | |||
| if segment_name == segments["name"]: | |||
| if segment_name == segment["name"]: | |||
There was a problem hiding this comment.
this was probably wrong...?
There was a problem hiding this comment.
Could you please also change segments["id"] below?
slicerio/segmentation.py
Outdated
| multiple_layers = True | ||
| elif header_key == "kinds": | ||
| if header[header_key] == ["domain", "domain", "domain"]: | ||
| multiple_layers = False # noqa: F841 |
There was a problem hiding this comment.
If we don't use multiple_layers then it is probably better to convert this to a comment than adding noqa
| @@ -689,7 +745,7 @@ def terminology_entry_matches(terminology1, terminology2): | |||
| def segment_id_from_name(segmentation, segment_name): | |||
| segments = segmentation["segments"] | |||
| for segment in segments: | |||
| if segment_name == segments["name"]: | |||
| if segment_name == segment["name"]: | |||
| @@ -689,7 +745,7 @@ def terminology_entry_matches(terminology1, terminology2): | |||
| def segment_id_from_name(segmentation, segment_name): | |||
| segments = segmentation["segments"] | |||
| for segment in segments: | |||
| if segment_name == segments["name"]: | |||
| if segment_name == segment["name"]: | |||
There was a problem hiding this comment.
Could you please also change segments["id"] below?
slicerio/server.py
Outdated
| if not auto_start: | ||
| raise | ||
|
|
||
| if retry_after_starting_server: | ||
| # Try again, with starting a server first | ||
| server_process = start_server(slicer_executable) | ||
| server_process = start_server(slicer_executable) # noqa: F841 |
There was a problem hiding this comment.
server_process is not used
There was a problem hiding this comment.
I removed server_process
There was a problem hiding this comment.
Ok, you can then it is better to delete server_process =
slicerio/tests/test_segmentation.py
Outdated
| }, | ||
| ] | ||
| ], |
There was a problem hiding this comment.
Many of these extra commas are actually misleading. The commas should be removed from all places where no additional list items are allowed. Extra line breaks make the code harder to read. It would be better to let the programmer decide where to break lines, as it is a result of conscious decisions not some simplistic rules about limiting line length. Same for indentation.
I don't want to ask you to undo all line break and indentation changes, as it could be a lot of work. But let's just undo those changes in this file and make sure the formatter does not try to change this anywhere (so I can later revert annoying changes).
There was a problem hiding this comment.
I can just undo the changes and disable the ruff-format hook, since you obviously don't like the black-style formatting.
|
@lassoan i think all comments are addressed. Do you want to have a look? Thanks |
add pre-commit
run pre-commit
pre-commit run --all-filesidentified possible bug(s):