Skip to content

update daft links #2169

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

Merged
merged 6 commits into from
Jul 9, 2025
Merged

update daft links #2169

merged 6 commits into from
Jul 9, 2025

Conversation

ccmao1130
Copy link
Contributor

Rationale for this change

We recently changed our site domain so want to update all Daft documentation links. And noticed that our package should be updated from getdaft to daft

Are these changes tested?

yes

Are there any user-facing changes?

no

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 fixing this @ccmao1130, we can move this forward as soon as the CI is green 👍

@@ -78,7 +78,7 @@ gcsfs = { version = ">=2023.1.0", optional = true }
huggingface-hub = { version = ">=0.24.0", optional = true }
psycopg2-binary = { version = ">=2.9.6", optional = true }
sqlalchemy = { version = "^2.0.18", optional = true }
getdaft = { version = ">=0.2.12", optional = true }
daft = { version = ">=0.5.0", optional = true }
Copy link
Contributor

Choose a reason for hiding this comment

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

Updating the minor version requires running poetry lock. What's the rationale for bumping to 0.5.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We changed our package from getdaft to daft and the earliest version for daft starts at v0.4.7 but we wanted to bump to v0.5.0

@kevinjqliu
Copy link
Contributor

@ccmao1130 could you run make install and then poetry lock locally? That should resolve the CI issue

@ccmao1130
Copy link
Contributor Author

@kevinjqliu Okay I think it should be okay now? (sorry not a developer hahaha....)

@kevinjqliu
Copy link
Contributor

Looks like something changed in the newer version

This test failed

@pytest.mark.integration
@pytest.mark.parametrize("catalog", [pytest.lazy_fixture("session_catalog_hive"), pytest.lazy_fixture("session_catalog")])
def test_daft_nan_rewritten(catalog: Catalog) -> None:
import daft
daft.context.set_runner_native()
table_test_null_nan_rewritten = catalog.load_table("default.test_null_nan_rewritten")
df = table_test_null_nan_rewritten.to_daft()
df = df.where(df["col_numeric"].float.is_nan())
df = df.select("idx", "col_numeric")
assert df.count_rows() == 1
assert df.to_pydict()["idx"][0] == 1
assert math.isnan(df.to_pydict()["col_numeric"][0])

___________________ test_daft_nan_rewritten[session_catalog] ___________________

catalog = local (<class 'pyiceberg.catalog.rest.RestCatalog'>)

    @pytest.mark.integration
    @pytest.mark.parametrize("catalog", [pytest.lazy_fixture("session_catalog_hive"), pytest.lazy_fixture("session_catalog")])
    def test_daft_nan_rewritten(catalog: Catalog) -> None:
        import daft
    
>       daft.context.set_runner_native()

tests/integration/test_reads.py:338: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

num_threads = None

    def set_runner_native(num_threads: int | None = None) -> DaftContext:
        """Configure Daft to execute dataframes using native multi-threaded processing.
    
        This is the default execution mode for Daft.
    
        Returns:
            DaftContext: Updated Daft execution context configured for native execution.
    
        Note:
            Can also be configured via environment variable: DAFT_RUNNER=native
        """
>       py_ctx = _set_runner_native(num_threads=num_threads)
E       daft.exceptions.DaftCoreException: DaftError::InternalError Cannot set runner more than once

../../../.cache/pypoetry/virtualenvs/pyiceberg-bKWKvoA4-py3.10/lib/python3.10/site-packages/daft/context.py:107: DaftCoreException

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.

CI is green now. LGTM!

@kevinjqliu kevinjqliu merged commit 479e663 into apache:main Jul 9, 2025
18 of 19 checks passed
@ccmao1130
Copy link
Contributor Author

Thanks Kevin!

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.

3 participants