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

Bump mypy version to ~=1.13.0 #6630

Merged
merged 10 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ notebook = [
]
pre-commit = [
'aiida-core[atomic_tools,rest,tests,tui]',
'mypy~=1.10.0',
'mypy~=1.13.0',
'packaging~=23.0',
'pre-commit~=3.5',
'sqlalchemy[mypy]~=2.0',
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/common/lang.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,4 @@ def __init__(self, getter: Callable[[SelfType], ReturnType]) -> None:
self.getter = getter

def __get__(self, instance: Any, owner: SelfType) -> ReturnType:
return self.getter(owner)
return self.getter(owner) # type: ignore[arg-type]
2 changes: 1 addition & 1 deletion src/aiida/common/pydantic.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ class Model(BaseModel):
field_info = Field(default, **kwargs)

for key, value in (('priority', priority), ('short_name', short_name), ('option_cls', option_cls)):
if value is not None:
if value is not None and field_info is not None:
Copy link
Member Author

Choose a reason for hiding this comment

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

Field seems can return None, if so the field_info won't have metadata attribute.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice fix. cc @edan-bainglass for visibility

field_info.metadata.append({key: value})

return field_info
2 changes: 1 addition & 1 deletion src/aiida/engine/processes/calcjobs/calcjob.py
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,7 @@ def presubmit(self, folder: Folder) -> CalcInfo:

def encoder(obj):
if dataclasses.is_dataclass(obj):
return dataclasses.asdict(obj)
return dataclasses.asdict(obj) # type: ignore[arg-type]
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure I should cast the type here, for the readability, I ignore the error.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would try to solve this in a subsequent PR

raise TypeError(f' {obj!r} is not JSON serializable')

subfolder = folder.get_subfolder('.aiida', create=True)
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/engine/processes/workchains/restart.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@


def validate_handler_overrides(
process_class: 'BaseRestartWorkChain', handler_overrides: Optional[orm.Dict], ctx: 'PortNamespace'
process_class: type['BaseRestartWorkChain'], handler_overrides: Optional[orm.Dict], ctx: 'PortNamespace'
danielhollas marked this conversation as resolved.
Show resolved Hide resolved
) -> Optional[str]:
"""Validator for the ``handler_overrides`` input port of the ``BaseRestartWorkChain``.

Expand Down
2 changes: 1 addition & 1 deletion src/aiida/manage/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ def create_runner(self, with_persistence: bool = True, **kwargs: Any) -> 'Runner
if with_persistence and 'persister' not in settings:
settings['persister'] = self.get_persister()

return runners.Runner(**settings)
return runners.Runner(**settings) # type: ignore[arg-type]

def create_daemon_runner(self, loop: Optional['asyncio.AbstractEventLoop'] = None) -> 'Runner':
"""Create and return a new daemon runner.
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/orm/nodes/data/array/projection.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ def array_list_checker(array_list, array_name, orb_length):
raise exceptions.ValidationError('Tags must set a list of strings')
self.base.attributes.set('tags', tags)

def set_orbitals(self, **kwargs):
def set_orbitals(self, **kwargs): # type: ignore[override]
Copy link
Member Author

Choose a reason for hiding this comment

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

I'll just leave kwargs as it is and ignore type checking.

Copy link
Collaborator

@danielhollas danielhollas Nov 23, 2024

Choose a reason for hiding this comment

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

Can you add a TODO comment with the error? The mypy error suggests that this is an actual error? Or do you think it's a false positive?

projection.py:281: error: Signature of "set_orbitals" incompatible with supertype "OrbitalData"  [override]
projection.py:281: note:      Superclass:
projection.py:281: note:          def set_orbitals(self, orbitals: Any) -> Any
projection.py:281: note:      Subclass:
projection.py:281: note:          def set_orbitals(self, **kwargs: Any) -> Any

Copy link
Member Author

@unkcpz unkcpz Nov 23, 2024

Choose a reason for hiding this comment

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

Neither, I think it is a design issue to use inheritance everywhere in the code base. Here it makes sense that the ProjectionData is only inheritant from ArrayData. For reduce the code duplication, can move the shared methods to dedicate functions or classes. It requires the actual use case of ProjectionData which I think it is in aiida-quantumespresso.

But since this method is just to override and block the method, so the type in the function call doesn't matter. Just ignore to skip the checker.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just merge it and if this is a problem make an issue out of it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I move this change to #6635, so let's continue the discussion there.

"""This method is inherited from OrbitalData, but is blocked here.
If used will raise a NotImplementedError
"""
Expand Down
4 changes: 2 additions & 2 deletions src/aiida/orm/nodes/data/list.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,15 @@ def remove(self, value):
self.set_list(data)
return item

def pop(self, **kwargs):
def pop(self, **kwargs): # type: ignore[override]
"""Remove and return item at index (default last)."""
data = self.get_list()
item = data.pop(**kwargs)
if not self._using_list_reference():
self.set_list(data)
return item

def index(self, value):
def index(self, value): # type: ignore[override]
"""Return first index of value.."""
return self.get_list().index(value)

Expand Down
4 changes: 1 addition & 3 deletions src/aiida/orm/nodes/data/singlefile.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,7 @@ def open(self, path: FilePath, mode: t.Literal['rb']) -> t.Iterator[t.BinaryIO]:

@t.overload
@contextlib.contextmanager
def open( # type: ignore[overload-overlap]
self, path: None = None, mode: t.Literal['r'] = ...
) -> t.Iterator[t.TextIO]: ...
def open(self, path: None = None, mode: t.Literal['r'] = ...) -> t.Iterator[t.TextIO]: ...

@t.overload
@contextlib.contextmanager
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/plugins/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def __init__(self):
def logger(self) -> Logger:
return self._logger

def get_version_info(self, plugin: str | type) -> dict[t.Any, dict[t.Any, t.Any]]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

@unkcpz FYI I think this has a similar issue that we discussed, I don't think adding Any here is correct. Let's discuss on the other PR, but just flagging it here as well so we don't forget.

Copy link
Collaborator

@danielhollas danielhollas Nov 25, 2024

Choose a reason for hiding this comment

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

The original error was:

src/aiida/plugins/utils.py:67: error: Subclass of "str" and "FunctionType" cannot exist: "FunctionType" is final [unreachable]
src/aiida/plugins/utils.py:67: error: Subclass of "type" and "FunctionType" cannot exist: "FunctionType" is final [unreachable]

Still struggling to understand, will need to experiment a bit.

def get_version_info(self, plugin: str | t.Any) -> dict[t.Any, dict[t.Any, t.Any]]:
"""Get the version information for a given plugin.

.. note::
Expand Down
2 changes: 1 addition & 1 deletion src/aiida/tools/pytest_fixtures/daemon.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def test(submit_and_await):
from aiida.engine import ProcessState

def factory(
submittable: 'Process' | 'ProcessBuilder' | 'ProcessNode',
submittable: type[Process] | ProcessBuilder | ProcessNode | t.Any,
state: ProcessState = ProcessState.FINISHED,
timeout: int = 20,
**kwargs,
Expand Down
2 changes: 1 addition & 1 deletion tests/cmdline/groups/test_dynamic.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Model(BaseModel):
union_type: t.Union[int, float] = Field(title='Union type')
without_default: str = Field(title='Without default')
with_default: str = Field(title='With default', default='default')
with_default_factory: str = Field(title='With default factory', default_factory=lambda: True)
with_default_factory: str = Field(title='With default factory', default_factory=lambda: True) # type: ignore[assignment]
Copy link
Member Author

Choose a reason for hiding this comment

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

It is test, so I just ignore it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, when I run mypy 1.13 on this, the # type: ignore[assignment] is not necessary?



def test_list_options(entry_points):
Expand Down
Loading