Skip to content
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

[CHORE] Run doctests in CI #2362

Merged
merged 9 commits into from
Jun 13, 2024
Merged

[CHORE] Run doctests in CI #2362

merged 9 commits into from
Jun 13, 2024

Conversation

colin-ho
Copy link
Contributor

@colin-ho colin-ho commented Jun 12, 2024

Add tests for our docs to CI

Currently the tests don't affect the CI checks because of the extra || true after the test command, but once all the examples have been fixed I'll remove it.

See the run here: https://github.com/Eventual-Inc/Daft/actions/runs/9490371825/job/26153717399?pr=2362

I also changed our daft.col example to make it work and show that it actually runs and is tested.
Screenshot 2024-06-12 at 3 47 16 PM
Screenshot 2024-06-12 at 3 47 25 PM

@github-actions github-actions bot added the chore label Jun 12, 2024
Example:
>>> df = daft.from_pydict({"a": [1.0, 2.2, 3.5, float("NaN")]})
>>> df.drop_null() # drops rows where any column contains Null/NaN values
>>> df = daft.from_pydict({"a": [1.6, 2.5, None, float("NaN")]})
>>> df.drop_null("a") # drops rows where column a contains Null/NaN values

Args:
Copy link
Contributor Author

@colin-ho colin-ho Jun 12, 2024

Choose a reason for hiding this comment

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

Made some small changes to drop_null and drop_nan because they had some errors that were preventing the dataframe.py file from being ran in doctest.

@@ -99,8 +99,8 @@ pub fn make_schema_vertical_table<F: AsRef<Field>>(fields: &[F]) -> comfy_table:
}

let header = vec![
comfy_table::Cell::new("Column Name").add_attribute(comfy_table::Attribute::Bold),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Had to remove the bolds here because we can't put bold characters in docstrings.

Copy link
Member

@samster25 samster25 Jun 12, 2024

Choose a reason for hiding this comment

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

Let's set up an environment variable / local config map for this! So we can set DAFT_BOLD_COLUMN_NAME_REPR=0 in the env

- name: Run doctests
run: |
source activate
maturin develop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if there's a way to cache the build from a previous test run?

Copy link
Member

Choose a reason for hiding this comment

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

Take a look at how we do rust-cache in the other workflows:

- uses: Swatinem/rust-cache@v2

cache: pip
cache-dependency-path: |
pyproject.toml
requirements-dev.txt
Copy link
Member

Choose a reason for hiding this comment

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

as said before: we have caching for other workflows

@colin-ho colin-ho enabled auto-merge (squash) June 13, 2024 15:45
@colin-ho colin-ho merged commit 647ec43 into main Jun 13, 2024
43 checks passed
@colin-ho colin-ho deleted the colin/doctests branch June 13, 2024 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants