Modifying dictionary?

[henryiii]

It looks like this modifies the input dictionary, filling in the defaults. That's why some checks are strongly failing for repo-review; repo-review assumes the input dictionary is not modified. There are three possible fixes:

1. Stop modifying the pyproject structure
2. Fix the repo-review check so that a deep copy is made before processing
3. Have repo-review give every check a deep copy

I'd much rather at least Option 2 over Option 3, but maybe Option 1 should be investigated first?

Before:

{'disallow_untyped_defs': False,
 'enable_error_code': ['ignore-without-code', 'redundant-expr', 'truthy-bool'],
 'files': ['src', 'web', 'tests'],
 'mypy_path': ['src'],
 'overrides': [{'disallow_untyped_defs': True, 'module': 'repo_review.*'}],
 'python_version': '3.10',
 'strict': True,
 'warn_unreachable': True,
 'warn_unused_configs': True}

After:

{'allow_redefinition': False,
 'allow_untyped_globals': False,
 'cache_dir': '.mypy_cache',
 'cache_fine_grained': False,
 'check_untyped_defs': False,
 'color_output': True,
 'disallow_any_decorated': False,
 'disallow_any_explicit': False,
 'disallow_any_expr': False,
 'disallow_any_generics': False,
 'disallow_any_unimported': False,
 'disallow_incomplete_defs': False,
 'disallow_subclassing_any': False,
 'disallow_untyped_calls': False,
 'disallow_untyped_decorators': False,
 'disallow_untyped_defs': False,
 'enable_error_code': ['ignore-without-code', 'redundant-expr', 'truthy-bool'],
 'error_summary': True,
 'explicit_package_bases': False,
 'extra_checks': False,
 'files': ['src', 'web', 'tests'],
 'follow_imports': 'normal',
 'follow_imports_for_stubs': False,
 'force_union_syntax': False,
 'force_uppercase_builtins': False,
 'hide_error_codes': False,
 'ignore_errors': False,
 'ignore_missing_imports': False,
 'implicit_optional': False,
 'implicit_reexport': True,
 'incremental': True,
 'local_partial_types': False,
 'mypy_path': ['src'],
 'namespace_packages': True,
 'no_implicit_optional': True,
 'no_implicit_reexport': False,
 'no_silence_site_packages': False,
 'no_site_packages': False,
 'overrides': [{'disallow_untyped_defs': True, 'module': 'repo_review.*'}],
 'pdb': False,
 'pretty': False,
 'python_version': '3.10',
 'raise_exceptions': False,
 'scripts_are_modules': False,
 'show_absolute_path': False,
 'show_column_numbers': False,
 'show_error_code_links': False,
 'show_error_codes': True,
 'show_error_context': False,
 'show_traceback': False,
 'skip_cache_mtime_checks': False,
 'skip_version_check': False,
 'sqlite_cache': False,
 'strict': True,
 'strict_concatenate': False,
 'strict_equality': False,
 'strict_optional': True,
 'verbosity': 0,
 'warn_incomplete_stub': False,
 'warn_no_return': True,
 'warn_redundant_casts': False,
 'warn_return_any': False,
 'warn_unreachable': True,
 'warn_unused_configs': True,
 'warn_unused_ignores': False}

[henryiii]

I was able to intelligently go with Option 3, where it only makes another copy if the input changes (at the expense of making at least one copy and doing a comparison every check). So now this will warn unless 1 or 2 is done, but will work correctly and not break other checks.

[abravalheri]

Thank you very much for reporting this @henryiii.

Probably this is related on how the fastjsonschema dependency works, and in that case it would be difficult to change.

A viable approach is to add a deepcopy in validate_pyproject directly, just before running the checks (there might be some small loss in terms of performance which could be counterbalanced if we make the deepcopy opt-in/opt-out).

Do you have any thoughts on that?

[henryiii]

Maybe it would be worth asking there first, then?

Documenting this and letting a user decide to pay the cost for the deepcopy would probably be fine - much of the time, it's probably fine to let it mutate, since often it's not being used after being validated. It's only needed if you want to use it afterwards.