Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable mccabe in ruff #636

Open
purva-thakre opened this issue Jun 24, 2024 · 4 comments
Open

Enable mccabe in ruff #636

purva-thakre opened this issue Jun 24, 2024 · 4 comments

Comments

@purva-thakre
Copy link
Collaborator

purva-thakre commented Jun 24, 2024

If I add C90 to the ruff tools:
https://docs.astral.sh/ruff/rules/#mccabe-c90

toqito/channel_ops/partial_channel.py:12:5: C901 `partial_channel` is too complex (12 > 10)
toqito/channels/partial_trace.py:11:5: C901 `partial_trace` is too complex (15 > 10)
toqito/channels/partial_transpose.py:11:5: C901 `partial_transpose` is too complex (12 > 10)
toqito/helper/channel_dim.py:8:5: C901 `channel_dim` is too complex (13 > 10)
toqito/helper/npa_hierarchy.py:63:5: C901 `_gen_words` is too complex (13 > 10)
toqito/helper/npa_hierarchy.py:125:5: C901 `npa_constraints` is too complex (19 > 10)
toqito/helper/tests/test_npa_hierarchy.py:12:5: C901 `cglmp_inequality` is too complex (12 > 10)
toqito/matrices/gell_mann.py:7:5: C901 `gell_mann` is too complex (11 > 10)
toqito/matrix_ops/tensor.py:6:5: C901 `tensor` is too complex (14 > 10)
toqito/matrix_props/sk_norm.py:18:5: C901 `sk_operator_norm` is too complex (27 > 10)
toqito/nonlocal_games/extended_nonlocal_game.py:123:9: C901 `nonsignaling_value` is too complex (25 > 10)
toqito/nonlocal_games/extended_nonlocal_game.py:294:9: C901 `__optimize_alice` is too complex (11 > 10)
toqito/nonlocal_games/nonlocal_game.py:363:9: C901 `__optimize_alice` is too complex (11 > 10)
toqito/nonlocal_games/nonlocal_game.py:473:9: C901 `nonsignaling_value` is too complex (25 > 10)
toqito/perms/permute_systems.py:12:5: C901 `permute_systems` is too complex (16 > 10)
toqito/state_props/is_product.py:59:5: C901 `_is_product` is too complex (11 > 10)
toqito/state_props/is_separable.py:20:5: C901 `is_separable` is too complex (32 > 10)

Adding the errors here to understand what is too complex later

@vprusso
Copy link
Owner

vprusso commented Jun 24, 2024

Interesting. Yes, the is_separable one makes sense as there are quite a few if conditions that need to be checked before the symmetric extension approach is invoked. Good to know!

@Shivansh20128
Copy link
Contributor

Yes. Here is the the reference for what too complex means. I checked the is_product function, and there are many if conditions there as well, so much so that at line 104, it enters the 3rd nested if condition. Maybe that is where the problem is. We could modify it to something simpler, or maybe we could find a way to change the threshold of McCabe complexity. This maybe relevant to that.

@vprusso
Copy link
Owner

vprusso commented Nov 17, 2024

Thanks for the references, @Shivansh20128 .

I think the McCabe complexity measure is a nice one, but similar to test coverage, I suppose I want to be careful that we aren't "chasing the wrong metric". It may very well be the case that some functions will simply have a high McCabe metric, and any refactoring to reduce that would have a net negative impact on the clarity of the code. It might be nice to hook this in as a diagnostic, but I'm not sure if we want to "live and die" by the numbers :). Does that make sense?

@Shivansh20128
Copy link
Contributor

Yes, suree! I just thought that since this issue is open maybe this is something you want to change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants