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

Expose library commands #45

Merged
merged 16 commits into from
Oct 16, 2024

Conversation

andrewdea
Copy link
Contributor

@andrewdea andrewdea commented Oct 11, 2024

Hello and thank you for this really cool library!

I've tried using this from my python code, but I realized that I needed to expose some of the internal commands.

I made a couple of changes, outlined below. If possible, I would also like to help expand this library so that it can cover other languages besides python.

outline of changes

  • create a Rust function code_complexity, that computes the complexity on a string of python code
  • expose this function at the level of the python library
    This way, we can evaluate the complexity on any string of code:
complexity.main.code_complexity(my_snippet_of_code)

This is particularly useful in situations where we want to evaluate stand-alone snippets that might not be part of a file-system.

  • I thought the Rust function cognitive_complexity was also really useful as a library command, so I exposed it at the python level. Because of Rust's naming rules, it seems it wouldn't allow me to keep that name: it would clash with the module of the same name. So I renamed the function function_complexity file_complexity, which I thought would clarify how it differs from code_complexity. I don't have a strong opinion on the function-names: feel free to propose better ones.

Let me know what you think and if there's any edits you'd like me to make before merging, thanks again!

Related issue: #46

@rohaquinlop
Copy link
Owner

Hey @andrewdea, thank you for your contribution!

This is a use case I didn't consider, and I find it really useful, so I appreciate these changes.

complexipy/main.py Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
complexipy/main.py Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
@rohaquinlop
Copy link
Owner

Can you please create an issue to relate this PR? And a new issue to discuss the new languages to support them.

@andrewdea andrewdea mentioned this pull request Oct 14, 2024
@andrewdea
Copy link
Contributor Author

@rohaquinlop thanks a lot for the comments!
I will go through your suggested changes tomorrow. In the meantime I have created issues:

@andrewdea
Copy link
Contributor Author

@rohaquinlop applied the suggested changes, let me know if I'm missing anything 😃

@rohaquinlop
Copy link
Owner

rohaquinlop commented Oct 15, 2024

The error at sdist pipeline is related to maturin-action #291, and it seems that can be solved using the ubuntu-22.04 image explicitly as mentioned in the issue

...
    runs-on: ubuntu-22.04
...

@andrewdea We will ignore that error in this PR, there are another pipelines that are failing due to ImportError's

@rohaquinlop
Copy link
Owner

rohaquinlop commented Oct 15, 2024

@andrewdea Can you please modify the CI.yml file and change this line?

...
  sdist:
    runs-on: ubuntu-22.04
    steps:
...

It seems that without that, the unit tests won't run 😢

Copy link

sonarcloud bot commented Oct 15, 2024

Copy link
Owner

@rohaquinlop rohaquinlop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The macOS pipeline is failing due to an error in the CI I'll fix that later, thank you so much for your contribution 🫶🏾

@rohaquinlop rohaquinlop merged commit d283084 into rohaquinlop:main Oct 16, 2024
12 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants