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

test: add dask-awkward to at least one of our tests. #2739

Merged
merged 4 commits into from
Oct 5, 2023

Conversation

jpivarski
Copy link
Member

We had one special test, Linux-ROOT, for testing ROOT (which is based on conda). Rather than add another test runner, I extended Linux-ROOT to Linux-ROOT-dask-awkward, to test both third-party interactions.

I believe that the set of issues we could face with ROOT and the set of issues we could face with dask-awkward are disjoint—I don't think that ROOT or Dask is augmented in a relevant way by the presence of the other one. (Maybe ROOT will notice that Dask is present and adjust one of its backends, but I don't see how that could affect our tests of ak.to_rdataframe and ak.from_rdataframe.)

@jpivarski
Copy link
Member Author

The branch protection requirement on Linux-ROOT will need to be replaced with Linux-ROOT-dask-awkward after everything passes and we're ready to merge (because all of the other PRs will see the change in requirements, and they'll want something to merge from main to adjust for it).

But I couldn't just leave the name as Linux-ROOT when it's also testing dask-awkward. Maybe it should be called Linux-third-party?

@codecov
Copy link

codecov bot commented Oct 4, 2023

Codecov Report

Merging #2739 (f6b6d57) into main (6dee1ea) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

@jpivarski jpivarski temporarily deployed to docs-preview October 4, 2023 21:33 — with GitHub Actions Inactive
@agoose77 agoose77 temporarily deployed to docs-preview October 4, 2023 23:22 — with GitHub Actions Inactive
@agoose77
Copy link
Collaborator

agoose77 commented Oct 5, 2023

@jpivarski is this finished? Does it just include dask-awkward as a dependency that is unused, but affects the environment through entrypoints?

@agoose77 agoose77 temporarily deployed to docs-preview October 5, 2023 19:08 — with GitHub Actions Inactive
@jpivarski
Copy link
Member Author

That's right: I intended to set it up as a test that would fail to show what needs to be fixed, and then you fixed it, so it's done now.

Since we don't explicitly import dask-awkward in any of our tests, the only effect was through entrypoints. But the effect was causing the test to fail (which can happen if someone decides to run the tests after installing from an sdist), so it was an important effect.

I'll change the branch protection rules to know about the Linux-ROOTLinux-ROOT-dask-awkward name change, then merge this PR, and then we should merge main into other ongoing PRs.

@jpivarski jpivarski merged commit 6c5e182 into main Oct 5, 2023
36 checks passed
@jpivarski jpivarski deleted the jpivarski/add-dask-awkward-to-Linux-ROOT-CI branch October 5, 2023 21:01
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.

Custom pickler doesn't work in a conda environment (with Python 3.10, at least)
2 participants