Skip to content

Replace old %-formatted by f-strings #93

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.

Sign up for GitHub

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

Merged
Fokko merged 2 commits into from
Oct 23, 2023
Merged

Replace old %-formatted by f-strings #93

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f1dba19
Replace old %-formatted by f-strings and add flynt hook to auto-detec…
hussein-awala Oct 21, 2023
782a5f6
add --line-length arg to detect long lines
hussein-awala Oct 21, 2023
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
8 changes: 8 additions & 0 deletions .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,11 @@ repos:
]
additional_dependencies:
- tomli==2.0.1
- repo: https://github.com/ikamensh/flynt
Copy link
Copy Markdown
Contributor

@Fokko Fokko Oct 21, 2023

Choose a reason for hiding this comment

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

Instead of adding a new plugin, can you enable the check in Ruff? https://docs.astral.sh/ruff/rules/#flynt-fly

Copy link
Copy Markdown
Member Author

@hussein-awala hussein-awala Oct 21, 2023

Choose a reason for hiding this comment

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

Yes, I will check it.

Copy link
Copy Markdown
Member Author

@hussein-awala hussein-awala Oct 21, 2023

Choose a reason for hiding this comment

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

The implementation of Flynt in ruff is still in progress (astral-sh/ruff#2102), the current rule doesn't detect all the usages of the old styles, and I cannot provide the argument line-length to deal with long lines. WDYT?

Copy link
Copy Markdown
Contributor

@Fokko Fokko Oct 23, 2023

Choose a reason for hiding this comment

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

Thanks, I was unaware of that. I ran it locally, and it didn't pick up the violations that are caught here.

rev: 1.0.1
hooks:
- id: flynt
args:
# --line-length is set to a high value to deal with very long lines
- --line-length
- '99999'
2 changes: 1 addition & 1 deletion pyiceberg/io/pyarrow.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ def visit_pyarrow(obj: Union[pa.DataType, pa.Schema], visitor: PyArrowSchemaVisi
Raises:
NotImplementedError: If attempting to visit an unrecognized object type.
"""
raise NotImplementedError("Cannot visit non-type: %s" % obj)
raise NotImplementedError(f"Cannot visit non-type: {obj}")


@visit_pyarrow.register(pa.Schema)
Expand Down
4 changes: 2 additions & 2 deletions pyiceberg/schema.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -794,7 +794,7 @@ def visit(obj: Union[Schema, IcebergType], visitor: SchemaVisitor[T]) -> T:
Raises:
NotImplementedError: If attempting to visit an unrecognized object type.
"""
raise NotImplementedError("Cannot visit non-type: %s" % obj)
raise NotImplementedError(f"Cannot visit non-type: {obj}")


@visit.register(Schema)
Expand Down Expand Up @@ -862,7 +862,7 @@ def pre_order_visit(obj: Union[Schema, IcebergType], visitor: PreOrderSchemaVisi
Raises:
NotImplementedError: If attempting to visit an unrecognized object type.
"""
raise NotImplementedError("Cannot visit non-type: %s" % obj)
raise NotImplementedError(f"Cannot visit non-type: {obj}")


@pre_order_visit.register(Schema)
Expand Down
9 changes: 3 additions & 6 deletions tests/test_schema.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -877,24 +877,21 @@ def test_identifier_fields_fails(table_schema_nested_with_struct_key_map: Schema
with pytest.raises(ValueError) as exc_info:
Schema(*table_schema_nested_with_struct_key_map.fields, schema_id=1, identifier_field_ids=[23])
assert (
"Cannot add field zip as an identifier field: must not be nested in %s"
% table_schema_nested_with_struct_key_map.find_field("location")
f"Cannot add field zip as an identifier field: must not be nested in {table_schema_nested_with_struct_key_map.find_field('location')}"
in str(exc_info.value)
)

with pytest.raises(ValueError) as exc_info:
Schema(*table_schema_nested_with_struct_key_map.fields, schema_id=1, identifier_field_ids=[26])
assert (
"Cannot add field x as an identifier field: must not be nested in %s"
% table_schema_nested_with_struct_key_map.find_field("points")
f"Cannot add field x as an identifier field: must not be nested in {table_schema_nested_with_struct_key_map.find_field('points')}"
in str(exc_info.value)
)

with pytest.raises(ValueError) as exc_info:
Schema(*table_schema_nested_with_struct_key_map.fields, schema_id=1, identifier_field_ids=[17])
assert (
"Cannot add field age as an identifier field: must not be nested in an optional field %s"
% table_schema_nested_with_struct_key_map.find_field("person")
f"Cannot add field age as an identifier field: must not be nested in an optional field {table_schema_nested_with_struct_key_map.find_field('person')}"
in str(exc_info.value)
)

Expand Down