Add mypy as a pre-commit hook to enforce better typing

[NP4567-dev]

Description

Adding mypy as a pre-commit hook is a good way to ensure better code quality and enforce best practices in my opinion. The first mypy all red error list is going to sting a bit but everything should be smoother afterwards. I can work on it as soon as you deem it a good idea.

Also, as this is not a adding a "maintainability" or "dev" labels for issues that are more technical/ci-related than new features?

[adrienbanse]

Hey @NP4567-dev, good idea, I don't see any reason not to try it :) I think that most of our functions are already typed, so it shouldn't be too much work

[samuelrince]

To answer the question about the tags "maintenance" or "dev" I think you (as dev on the project) should be able to create a blank issue instead of using the feature/bug templates? Tell me if it is not the case!

And I am okay with the integration of mypy. I have not used it a lot, to be honest, but still think it is good practice to have it. I know that there are other alternatives as well like Pyright or Pyre, but again, not very familiar to me.

[NP4567-dev]

I'm a contributor but not a dev I think? You have to manually add me to the project members. It does not seem like I can edit labels. No worries, I'm ok with developping on my fork.

Currently mypy detects more than twenty issues. Some are pretty trivial to handle but solving others mean a lot of code refactoring. I can try to split them in issues that could be handled individually but everything might be intricated.

This should be a gain in the long run as it should make the code more maintainable but I don't want to touch anything that is currently being updated in another issue or break something that was done for reasons I don't get.

Let me know how you'd like to handle this.

[adrienbanse]

Personally I'm ok with putting a bit of effort to make it work. @samuelrince If you agree let's open these issues.

[samuelrince]

Yes, I am in favor of adding mypy! I'd rather have it in the same issue/PR if that's possible?

If @NP4567-dev you already have a prototype (even non-working) it would be great if you can push it in new branch in this repo! Ping me on Slack if you have permission issues, but it should be fine.

[adrienbanse]

Hey @NP4567-dev, the remaining errors may be linked with @jaywonchung's comment in https://github.com/genai-impact/ecologits/issues/95:

EcoLogits has excellent typing, but by monkeypatching the impacts instance attribute into API client response objects (e.g., ChatCompletion), it makes type checkers like Mypy and Pyright complain about missing attributes. I am very much aware that dynamic monkeypatching and static type checkers do not work well with each other, but in any case I think it would be nice to investigate. If this is infeasible, noting somewhere in the documentation about workarounds (e.g., typing.cast to ecologits.tracers.openai_tracer.ChatCompletion) could be helpful.

[NP4567-dev]

Ok, so I finally had some time and have a first draft on my fork here

Not really fixed:

• This is a very crude implementation as a lot of errors are ignored. You can remove all the #type: ignore to check and run pre-commit run --all-files to get all the original messages.

Updated:

• I removed some "Optional" typings that were applied on values with default provided as this was causing typing problems down the line
• In instead of returning None, llm_impacts now raises error for missing model or electricity mix. I added try catch on all tracers, but I think building an abstract class around tracers could make less repetition in code. --> this needs some tests added.
• I removed the @Override decorator as they were detected as not overriding anything by mypy, but this could be handled with ignore if you deem important to have them.