You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
While updating dependencies, PR #91 also reworked the order of click decorators to address a new, external conflict between click==8.1.3 and mypy==1.4.1.
This underlying conflict was discovered after the code change, and it's assumed a fix will be rolled out to click in the near future that will allow the previous decorator order while also aligning with mypy linting, as many others have encountered this as well.
Proposing to revert the click decorators to match other projects (with @click.command as the top-most decorator, with others following) once it's been determined that click and mypy are aligned.
Current:
@click.option("-i","--input-file",required=True,help="Filepath for harvested input records to transform",)@click.option("-o","--output-file",required=True,help="Filepath to write output TIMDEX JSON records to",)@click.option("-s","--source",required=True,type=click.Choice(list(SOURCES.keys()), case_sensitive=False),help="Source records were harvested from, must choose from list of options",)@click.option("-v", "--verbose", is_flag=True, help="Pass to log at debug level instead of info")@click.command()defmain(source, input_file, output_file, verbose):
...
Proposed reverted ordering, matching convention in other projects:
@click.command()@click.option("-i","--input-file",required=True,help="Filepath for harvested input records to transform",)@click.option("-o","--output-file",required=True,help="Filepath to write output TIMDEX JSON records to",)@click.option("-s","--source",required=True,type=click.Choice(list(SOURCES.keys()), case_sensitive=False),help="Source records were harvested from, must choose from list of options",)@click.option("-v", "--verbose", is_flag=True, help="Pass to log at debug level instead of info")defmain(source, input_file, output_file, verbose):
...
The text was updated successfully, but these errors were encountered:
* Update type hints in helpers.write_timdex_records_to_json
Why these changes are being introduced:
* After some refactoring in 07/2022 that added an iterator pattern to Transformer, it looks as though downstream type hints were not updated.
* While this did not affect runtime, it made reading the code somewhat confusing to someone new to the codebase.
How this addresses that need:
* Update the `records` input argument of `helpers.write_timdex_records_to_json()` from `Iterator[TimdexRecord]` to `Transformer`
* rename `records` to `transformer_instance`, matching variable names from calling `cli.py`
* Add explicit `TimdexRecord` type hint for result of `next(transformer_instance)`
Side effects of this change:
* None
Relevant ticket(s):
* None
Additional maintenance:
* Updated syntax for isort command in Makefile
* Based on new results from isort, update import order in helpers.py
* update dependencies and revert click workaround
* In PR #91, while updating dependencies, `click` and `mypy` had a conflict.
* A temporary workaround was put into place that reordered the `click` decorators, but this deviated from other projects.
* Now that `click` has updated again, that workaround is no longer needed.
* Resolves#92
While updating dependencies, PR #91 also reworked the order of
click
decorators to address a new, external conflict betweenclick==8.1.3
andmypy==1.4.1
.This underlying conflict was discovered after the code change, and it's assumed a fix will be rolled out to
click
in the near future that will allow the previous decorator order while also aligning withmypy
linting, as many others have encountered this as well.Proposing to revert the
click
decorators to match other projects (with@click.command
as the top-most decorator, with others following) once it's been determined thatclick
andmypy
are aligned.Current:
Proposed reverted ordering, matching convention in other projects:
The text was updated successfully, but these errors were encountered: