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 (fcf450f).
Report is 145 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    42148     +704     
==========================================
+ Hits        32120    32832     +712     
+ Misses       9324     9316       -8

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

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.

.github/workflows/ci-code.yml

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?

This was excluding files docs/source/topics/processes/include/snippets/functions/parse_docstring_expose_ipython.py and docs/source/topics/processes/include/snippets/functions/signature_plain_python_call_illegal.py from linting and formatting.

I've moved this configuration to pyproject.toml which I think is a much better place for it. For example, you can now run ruff also manually outside of pre-commit and it will have the same configuration.

Ah this is reusing a yaml anchor! thanks!

What you said, yes 😅

FTR: I am refusing to google what is "yaml anchor" :-D

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

unkcpz · 2024-11-26T09:35:23Z

@danielhollas I put this into "v2.7.0" in case there are some changes that we are not sure and will break the backward compatibility. Otherwise we can just have this for a patch release. What do you think?

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

There shouldn't be any breaking changes, but at the same time this certainly doesn't contain any bugfixes so I don't see a reason why it would need to go in a patch release. v2.7.0 is fine

agoscinski

I also don't see any reason for keeping it outside of a patch release if it simplifies the creation of it because of merge conflicts

unkcpz

Looks all good! Thanks @danielhollas

danielhollas · 2024-11-27T15:24:16Z

I also don't see any reason for keeping it outside of a patch release if it simplifies the creation of it because of merge conflicts

True, there's a fair amount of churn in this PR due to import reorderings. In case you thought it important, I could split this into two commits.

EDIT: sorry, didn't mean to close the PR.

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

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

Bump ruff version

7be553f

agoscinski force-pushed the bump-ruff branch from ece43c7 to 7be553f Compare November 25, 2024 16:28

agoscinski approved these changes Nov 26, 2024

View reviewed changes

Merge branch 'main' into bump-ruff

fcf450f

unkcpz approved these changes Nov 27, 2024

View reviewed changes

danielhollas closed this Nov 27, 2024

danielhollas reopened this Nov 27, 2024

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

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

danielhollas Nov 25, 2024 •

edited

Loading

agoscinski Nov 26, 2024

danielhollas Nov 26, 2024

danielhollas Nov 26, 2024

agoscinski Nov 25, 2024

agoscinski Nov 25, 2024

unkcpz commented Nov 26, 2024

danielhollas commented Nov 26, 2024

agoscinski left a comment

unkcpz left a comment

danielhollas commented Nov 27, 2024 •

edited

Loading

		@@ -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

agoscinski left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danielhollas Nov 25, 2024 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

unkcpz commented Nov 26, 2024

danielhollas commented Nov 26, 2024

agoscinski left a comment

Choose a reason for hiding this comment

unkcpz left a comment

Choose a reason for hiding this comment

danielhollas commented Nov 27, 2024 • edited Loading

danielhollas commented Nov 18, 2024 •

edited

Loading

codecov bot commented Nov 18, 2024 •

edited

Loading

agoscinski left a comment •

edited

Loading

danielhollas Nov 25, 2024 •

edited

Loading

danielhollas commented Nov 27, 2024 •

edited

Loading