Bump ruff version #6614

danielhollas · 2024-11-18T10:40:28Z

Most of the changes here are minor import sorting changes (I checked all of them and they all look more correct than before).

The only other change were couple type-comparison (E721) fixes, these should be reviewed carefully.

I also turned off verbose pytest output in CI since it produces thousands of lines and is very difficult to navigate in Github UI.

type-comparison (E721)

Derived from the pycodestyle linter.

What it does

Checks for object type comparisons using == and other comparison
operators.

Why is this bad?

Unlike a direct type comparison, isinstance will also check if an object
is an instance of a class or a subclass thereof.

If you want to check for an exact type match, use is or is not.

Known problems

When using libraries that override the == (__eq__) operator (such as NumPy,
Pandas, and SQLAlchemy), this rule may produce false positives, as converting
from == to is or is not will change the behavior of the code.

For example, the following operations are not equivalent:

import numpy as np

np.array([True, False]) == False
# array([False,  True])

np.array([True, False]) is False
# False

Example

if type(obj) == type(1):
    pass

if type(obj) == int:
    pass

Use instead:

if isinstance(obj, int):
    pass

codecov · 2024-11-18T11:08:49Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.90%. Comparing base (ef60b66) to head (ece43c7).
Report is 136 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6614      +/-   ##
==========================================
+ Coverage   77.51%   77.90%   +0.40%     
==========================================
  Files         560      567       +7     
  Lines       41444    42147     +703     
==========================================
+ Hits        32120    32831     +711     
+ Misses       9324     9316       -8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests

danielhollas · 2024-11-18T11:15:26Z

docs/source/topics/workflows/include/snippets/expose_inputs/complex_parent.py

@@ -1,6 +1,7 @@
-from aiida.engine import ToContext, WorkChain
 from child import ChildWorkChain


This rearrangement is not that great, but whatever.

danielhollas · 2024-11-18T13:38:43Z

.github/workflows/ci-code.yml

@@ -104,7 +104,7 @@ jobs:
        AIIDA_WARN_v3: 1
      # Python 3.12 has a performance regression when running with code coverage
      # so run code coverage only for python 3.9.
-      run: pytest -v tests -m 'not nightly' ${{ matrix.python-version == '3.9' && '--cov aiida' || '' }}


Running pytest in verbose mode produces more than 2000 lines of output, which is extremely annoying in Github UI.

danielhollas · 2024-11-18T13:41:07Z

src/aiida/cmdline/groups/dynamic.py

@@ -177,7 +177,7 @@ def list_options(self, entry_point: str) -> list:
            # ``typing.Union[str, None].__args__`` will return the tuple ``(str, NoneType)``. So to get the real type,
            # we simply remove all ``NoneType`` and the remaining type should be the type of the option.
            if hasattr(field_info.annotation, '__args__'):
-                args = list(filter(lambda e: e != type(None), field_info.annotation.__args__))


This is one of the few changes, I think this is correct, but please check.

@edan-bainglass I think you familiar with this part?

The only way a different result can be achieved is when a custom type overwrites the equal function. That is not the case for NoneType, so it is ok.

danielhollas · 2024-11-18T13:41:40Z

src/aiida/engine/processes/workchains/workchain.py

@@ -208,7 +208,7 @@ def _resolve_nested_context(self, key: str) -> tuple[AttributeDict, str]:
            #   (subclasses of AttributeDict) but after resolution of an Awaitable this will be the value itself
            # * assumption: a resolved value is never a plain AttributeDict, on the other hand if a resolved Awaitable
            #   would be an AttributeDict we can append things to it since the order of tasks is maintained.
-            if type(ctx) != AttributeDict:


AttributeDict is a class inheriting from dict so the __eq__ function of the type is not overwritten so it is okay here

agoscinski

I think you need to wait until #6630 is merged to fix the CI. Otherwise looks good to me

agoscinski · 2024-11-25T12:44:09Z

.pre-commit-config.yaml

  - id: ruff
-    exclude: *exclude_ruff


do you know what this did?

agoscinski · 2024-11-25T13:04:07Z

src/aiida/cmdline/groups/dynamic.py

@@ -177,7 +177,7 @@ def list_options(self, entry_point: str) -> list:
            # ``typing.Union[str, None].__args__`` will return the tuple ``(str, NoneType)``. So to get the real type,
            # we simply remove all ``NoneType`` and the remaining type should be the type of the option.
            if hasattr(field_info.annotation, '__args__'):
-                args = list(filter(lambda e: e != type(None), field_info.annotation.__args__))


The only way a different result can be achieved is when a custom type overwrites the equal function. That is not the case for NoneType, so it is ok.

agoscinski · 2024-11-25T13:07:04Z

src/aiida/engine/processes/workchains/workchain.py

@@ -208,7 +208,7 @@ def _resolve_nested_context(self, key: str) -> tuple[AttributeDict, str]:
            #   (subclasses of AttributeDict) but after resolution of an Awaitable this will be the value itself
            # * assumption: a resolved value is never a plain AttributeDict, on the other hand if a resolved Awaitable
            #   would be an AttributeDict we can append things to it since the order of tasks is maintained.
-            if type(ctx) != AttributeDict:


AttributeDict is a class inheriting from dict so the __eq__ function of the type is not overwritten so it is okay here

danielhollas force-pushed the bump-ruff branch from b16a1ae to a00d50d Compare November 18, 2024 11:38

danielhollas commented Nov 18, 2024

View reviewed changes

danielhollas requested a review from agoscinski November 18, 2024 13:48

danielhollas marked this pull request as ready for review November 18, 2024 13:48

danielhollas mentioned this pull request Nov 23, 2024

Bump ruff to v0.8 #6634

Draft

Bump ruff version

ece43c7

danielhollas force-pushed the bump-ruff branch from e36d075 to ece43c7 Compare November 25, 2024 11:17

agoscinski reviewed Nov 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bump ruff version #6614

Bump ruff version #6614

danielhollas commented Nov 18, 2024 •

edited

Loading

codecov bot commented Nov 18, 2024 •

edited

Loading

danielhollas Nov 18, 2024

danielhollas Nov 18, 2024

danielhollas Nov 18, 2024

unkcpz Nov 18, 2024

agoscinski Nov 25, 2024

danielhollas Nov 18, 2024

agoscinski Nov 25, 2024

agoscinski left a comment •

edited

Loading

agoscinski Nov 25, 2024

agoscinski Nov 25, 2024

agoscinski Nov 25, 2024

		@@ -1,6 +1,7 @@
		from aiida.engine import ToContext, WorkChain
		from child import ChildWorkChain

Bump ruff version #6614

Are you sure you want to change the base?

Bump ruff version #6614

Conversation

danielhollas commented Nov 18, 2024 • edited Loading

type-comparison (E721)

What it does

Why is this bad?

Known problems

Example

codecov bot commented Nov 18, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agoscinski left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielhollas commented Nov 18, 2024 •

edited

Loading

codecov bot commented Nov 18, 2024 •

edited

Loading

agoscinski left a comment •

edited

Loading