Skip to content

Conversation

@correctmost
Copy link
Contributor

PR Description:

This PR reduces the number of Any instances in the code. I think mypy is now catching a real bug in LvmConfiguration.parse_arg too.

@correctmost correctmost requested a review from Torxed as a code owner November 25, 2024 10:08
return config

for entry in disk_config.get('device_modifications', []):
device_path = Path(entry.get('device', None)) if entry.get('device', None) else None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed the conditional get to avoid this error:

error: Argument 1 to "Path" has incompatible type "str | None"; expected "str | PathLike[str]"  [arg-type]

return {
'value': self.value,
'unit': self.unit.name,
'sector_size': self.sector_size.json() if self.sector_size else None
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't seem like self.sector_size can be None given the __post_init__ code above. I changed the code to avoid this mypy error:

error: Incompatible types (expression has type "_SectorSizeSerialization | None", TypedDict item "sector_size" has type "_SectorSizeSerialization")  [typeddict-item]

for part in mod.partitions:
if part.obj_id in arg.get('lvm_pvs', []):
# FIXME: 'lvm_pvs' does not seem like it can ever exist in the 'arg' serialization
if part.obj_id in arg.get('lvm_pvs', []): # type: ignore[operator]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this arg.get('lvm_pvs', []) call can ever succeed.

It looks like the LvmVolumeGroup data needs to be checked instead.

@svartkanin svartkanin merged commit 007f2ff into archlinux:master Nov 30, 2024
8 checks passed
@correctmost correctmost deleted the cm/add-typeddict-for-device-model branch November 30, 2024 11:59
castillofrancodamian pushed a commit to castillofrancodamian/archinstall that referenced this pull request Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants