Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

@Fokko
Copy link
Contributor

@Fokko Fokko commented Nov 1, 2024

No description provided.

tests/test_schema.py Outdated Show resolved Hide resolved
pyiceberg/table/update/schema.py Outdated Show resolved Hide resolved
assert isinstance(applied.fields[0].field_type, DoubleType)


def test_promote_long_to_int() -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

nit, I dont see a test case for int to long. Can we add that here?

Theres a corresponding test case for double to float and float to double

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good one, added those as well 👍

tests/test_schema.py Outdated Show resolved Hide resolved
Fokko and others added 7 commits November 4, 2024 15:03
Co-authored-by: Kevin Liu <kevinjqliu@users.noreply.github.com>
Co-authored-by: Kevin Liu <kevinjqliu@users.noreply.github.com>
Copy link
Contributor

@kevinjqliu kevinjqliu left a comment

Choose a reason for hiding this comment

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

LGTM! A few nit comments

tests/test_schema.py Show resolved Hide resolved
tests/test_schema.py Outdated Show resolved Hide resolved
@Fokko Fokko merged commit c3bf16c into apache:main Nov 5, 2024
7 checks passed
@Fokko Fokko deleted the fd-extend-union branch November 5, 2024 15:54
@Fokko
Copy link
Contributor Author

Fokko commented Nov 5, 2024

Thanks for the review @kevinjqliu 🙌

sungwy pushed a commit to sungwy/iceberg-python that referenced this pull request Dec 7, 2024
* Allow union of `{int,long}`, `{float,double}`, etc

* Thanks Kevin!

Co-authored-by: Kevin Liu <kevinjqliu@users.noreply.github.com>

* Thanks Kevin!

Co-authored-by: Kevin Liu <kevinjqliu@users.noreply.github.com>

* MOAR tests

* lint

* Make the tests happy

* Remove redundant test

---------

Co-authored-by: Kevin Liu <kevinjqliu@users.noreply.github.com>
sungwy pushed a commit to sungwy/iceberg-python that referenced this pull request Dec 7, 2024
* Allow union of `{int,long}`, `{float,double}`, etc

* Thanks Kevin!

Co-authored-by: Kevin Liu <kevinjqliu@users.noreply.github.com>

* Thanks Kevin!

Co-authored-by: Kevin Liu <kevinjqliu@users.noreply.github.com>

* MOAR tests

* lint

* Make the tests happy

* Remove redundant test

---------

Co-authored-by: Kevin Liu <kevinjqliu@users.noreply.github.com>
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

Morty Proxy This is a proxified and sanitized view of the page, visit original site.