Fix pre-commit errors (#192)

[leila-pujal]

This PR addresses the request posted on issue #192 and includes the missing parts of PR #151 that were not included in #137. This also relates to PR #187.

Checklist

• Write a good description of what the PR does.
• Add tests for each unit of code added (e.g. function, class)
• Update documentation
• Squash commits that can be grouped together
• Rebase onto master

Type of Changes

	Type
✓	📜 Docs

Related

[leila-pujal]

Hi @tovrstra, I made the PR to include all the errors from running pre-commit run --all. I was able to fix most of them but there are some @msricher and me are unsure about how impactful they are. I list them below:

gbasis/integrals/libcint.py:185:16: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
gbasis/integrals/libcint.py:194:16: RUF012 Mutable class attributes should be annotated with `typing.ClassVar`
gbasis/integrals/libcint.py:314:17: B904 Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling
gbasis/integrals/libcint.py:811:21: RUF005 Consider iterable unpacking instead of concatenation
gbasis/integrals/libcint.py:1236:13: B904 Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` to distinguish them from errors in exception handling

@msricher commented that we don't use any annotation in Gbasis (I think IOData and Grid does) and then the one we are more unsure about it is the B904 with the error exception. Any thoughts on how we should handle these would be super useful. Thanks!

[tovrstra]

Ruff has recommendations for every error message it can generate and these are usually quite informative: https://docs.astral.sh/ruff/rules/

I'll try to give some advice for each type of problem;

RUF012: annotations are generally a useful thing, but I can understand you don't want to get into it now. Adding them everywhere for consistency is a lot of work and outside this scope of this PR. You may try making the _fields_ attribute a tuple instead of a list, e.g. as in the following example from the ctypes documentation:

class POINT(Structure):
    _fields_ = ("x", c_int), ("y", c_int)

I'm not sure if that will solve the Ruff message, but it is a useful change anyway. There is no reason to have a mutable _fields_ attribute. Feel free to add an ignore line to pyproject.toml for RUF012 if it keeps complaining.

B904: In both cases, it would suggest deviating from the EAFP pattern, i.e. something as follows instead:

cfunc = self._cache.get(attr)
if cfunc is None:
    cfunc = self._get_cfunc(attr)
    self._cache[attr] = cfunc
return cfunc

Inside the helper method _get_cfunc, you can raise the exception if needed. As a side effect, the linter message will go away. I don't follow the argument that EAFP always leads to more readable code. In simple examples, it sometimes does. By working with the get method, you also address the EAFP argument that it reduces the number of dictionary lookups.

RUF005: Can be fixed as out_shape = (self.nbfn, self.nbfn, self.nbfn, self.nbfn, *components). See https://docs.astral.sh/ruff/rules/collection-literal-concatenation/

[tovrstra]

P.S. If you prefer to stick to EAPF, which is certainly fine, then you can fix B904 as follows:

raise ValueError(f"there is no ``gbasis`` API for the function {attr}") from None`

The more elaborate option would be:

try:
     cfunc = self._cache[attr]
except KeyError as exc:
    ...
    else:
       raise ValueError(f"there is no ``gbasis`` API for the function {attr}") from exc

The second form seems less suited here because the KeyError is related to the caching and will not offer useful information.