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

Update algorithm #40

Closed
rohaquinlop opened this issue Apr 4, 2024 · 12 comments
Closed

Update algorithm #40

rohaquinlop opened this issue Apr 4, 2024 · 12 comments
Assignees
Labels
bug Something isn't working

Comments

@rohaquinlop
Copy link
Owner

Python let the users make use of list comprehension, this isn't contemplated in the implementation,

@rohaquinlop rohaquinlop added the bug Something isn't working label Apr 4, 2024
@rohaquinlop rohaquinlop self-assigned this Apr 4, 2024
@rohaquinlop rohaquinlop reopened this May 29, 2024
@rohaquinlop rohaquinlop changed the title Upgrade algorithm Update algorithm May 29, 2024
@rohaquinlop
Copy link
Owner Author

rohaquinlop commented May 29, 2024

Complexity: 20

def nested_dict(
    *,
    data: IndexedDict,
    keys: list[str] | str,
    keys_node: Node,
    value: Any | None = None,
    value_node: Node | None = None,
) -> None:
    build_key = []

    for _, is_last, key in mark_ends(keys if isinstance(keys, list) else [keys]):
        build_key.append(key)
        data = _nested_dict_handle_list(data)

        if is_last and key in data:
            raise DuplicatedKey(f"Defining a key multiple times is invalid: {key}")

        if key not in data:
            if is_last:
                if key in data:
                    raise DuplicatedKey(
                        f"Defining a key multiple times is invalid: {key}"
                    )
                data[(key, keys_node)] = (
                    value if value is not None else IndexedDict(),
                    value_node or keys_node,
                )
                continue
            data[(key, keys_node)] = (IndexedDict(), value_node or keys_node)
        data = data[key]

image
image

@rohaquinlop
Copy link
Owner Author

Also, consider the list comprehension cases

@rohaquinlop
Copy link
Owner Author

image

rohaquinlop added a commit that referenced this issue Jun 21, 2024
- Update the cognitive complexity algorithm to consider
cases that were not previously considered.
- In this commit, the algorithm was updated to consider
the cognitive complexity of the following expressions:
  - Call
  - Tuple
  - List
  - Set
  - Dict
- The algorithm was also updated to consider the nesting
level of the expression when calculating the cognitive
complexity of the expression. This change was made to
ensure that the cognitive complexity of the expression
is calculated correctly due to the python syntax.
- This commit also updates the version of the project to
0.4.0.
rohaquinlop added a commit that referenced this issue Jun 21, 2024
feat(back): #40 update cognitive complexity algorithm
@andrewdea
Copy link
Contributor

Hi @rohaquinlop was this fixed or does it still need some work?
I noticed this behavior in the algorithm:
This snippet gets a complexity of 1:

>>> snippet = """for x in range(0, 10):
    print("even" if x % 2 == 0 else "odd")
"""

>>> cc = code_complexity(snippet)
>>> cc.complexity
1

But this snippet gets a complexity of 3:

>>> snippet = """for x in range(0, 10):
    if x % 2 == 0:
        print("even")
    else:
        print("odd")
"""

>>> cc = code_complexity(snippet)
>>> cc.complexity
3

Is this expected? I could see an argument for treating the inline if-statements as "less complex" but I'm not sure if this was discussed in the paper. Happy to help with this if needed

@rohaquinlop
Copy link
Owner Author

At least for me, both must have the same complexity, this is a case that I didn't consider, should we check each parameter of a function call? 🤔

@andrewdea
Copy link
Contributor

yeah I agree with you. I'll take another look at the paper to make sure that is the authors' intent as well.
I think python is particularly tricky here: pretty much any expression could be an inline if-else statement, so I think we'll have to make the algorithm 'dig deeper' into the syntax-tree than it currently does.

@andrewdea
Copy link
Contributor

andrewdea commented Oct 28, 2024

Hey @rohaquinlop :)
I went back and re-read the paper: I think there is an argument for scoring the inline if-else statement as less complex.
The paper does not explicitly mention this python-specific syntax, and every time they mention an if-else statement they say it should increment the score. However, the first of the three "basic rules" of the score is:

Ignore structures that allow multiple statements to be readably shorthanded into one

The example they provide for this is the null-coalescing operator:

MyObj myObj = a?.myObj;

I'm still leaning towards incrementing inline if-else statements, but thought I'd mention this just so we clarify the implementation

@rohaquinlop
Copy link
Owner Author

rohaquinlop commented Oct 28, 2024

@andrewdea Then let's keep the current algorithm, I was thinking and isn't normal to nest a lot of expressions in a single inline expression so we can consider it as "no complex". If in the future we find that it is necessary to check this case then we'll add it, what do you think about that?

Also, I think that the new version's release will be today, I'm working to achieve that.

@andrewdea
Copy link
Contributor

@rohaquinlop makes sense to me, let's go with this for now!
It does seem like the authors of the paper left some wiggle-room to interpretation. And as someone who reads quite a bit of python, I'd say that inline statements are generally used in a way that keeps the added complexity to a minimum.

I do think it could be good to add some more documentation to specifically outline the details of our implementation: which statements get an increase in complexity and why. But that's not a very urgent concern.

And thanks so much for working on the release! looking forward to that :)

@andrewdea
Copy link
Contributor

@rohaquinlop should we go ahead and close this issue then?

@rohaquinlop
Copy link
Owner Author

@andrewdea I completely forget to announce you that the version 0.5.0 has been released: https://github.com/rohaquinlop/complexipy/releases/tag/0.5.0

@andrewdea
Copy link
Contributor

thanks @rohaquinlop I did see the release 🎉
Looks great and I really like your usability improvements in #52

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants