Skip to content

allow string transforms in add_field() #1883

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

Conversation

Federico-Vichachi
Copy link

@Federico-Vichachi Federico-Vichachi commented Apr 4, 2025

Closes #1882

Rationale for this change

This change allows the add_field method to accept str values as transform inputs, in addition to Transform instances. This improves developer experience by supporting more flexible input formats when defining transformations, and is consistent with similar string-based input handling elsewhere in the codebase.

Are there any user-facing changes?

No user-facing changes. This is an internal API enhancement that improves developer flexibility when working with partition transforms.

Copy link
Contributor

@Fokko Fokko left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up @Federico-Vichachi I left some small comments, and it would be nice to add a test.

It would be good to check locally if the tests pass using make lint and make test.

@@ -85,12 +86,16 @@ def __init__(self, transaction: Transaction, case_sensitive: bool = True) -> Non
def add_field(
self,
source_column_name: str,
transform: Transform[Any, Any],
transform: Union[str, Transform[Any, Any]], # Aceptamos strings o Transform
Copy link
Contributor

Choose a reason for hiding this comment

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

😉

Suggested change
transform: Union[str, Transform[Any, Any]], # Aceptamos strings o Transform
transform: Union[str, Transform[Any, Any]], # We accept strings or Transform

Comment on lines +136 to 137

def add_identity(self, source_column_name: str) -> UpdateSpec:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to revert this to make the linter happy:

Suggested change
def add_identity(self, source_column_name: str) -> UpdateSpec:
def add_identity(self, source_column_name: str) -> UpdateSpec:

@Federico-Vichachi
Copy link
Author

Hi @Fokko, I appreciate your corrections and will include them. Regarding the test, I was trying to run the following one but had problems installing the project's dependencies with poetry install. This is the test I want to run in test_partition_evolution.py.
update_spec = table.update_spec()
update_spec.add_field("example_col", "identity")
update_spec.commit()
assert "example_col" in table.schema().fields`

@Federico-Vichachi Federico-Vichachi deleted the handle-string-transform branch April 9, 2025 11:53
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.

Allow passing in a string transform for tbl.update_spec().add_field()
2 participants