fast_slow_proxy: Don't import assert_eq at top-level

[wence-]

Description

The testing._utils module imports pytest, which is not advertised as a default run dependency of cudf, so we must avoid importing it in the proxy wrappers at top-level.

Since what we need in the proxy wrappers for pandas debugging is the assert_eq function (which does not need pytest), move it to testing.testing (where it more naturally fits with the other assertion functions anyway). This removes the need for pytest when running the fast-slow-proxy wrappers.

• Closes [BUG] cuDF Pandas now requires pytest at runtime #16062.

Checklist

• I am familiar with the Contributing Guidelines.
• New or existing tests cover these changes.
• The documentation is up to date with these changes.

[bdice]

The assert_eq function does not use pytest. I think a better fix here is to rework the structure of testing/_utils.py and move assert_eq to a separate file that doesn't require pytest (or move the bits of testing/_utils.py that require pytest into another location).

[wence-]

The assert_eq function does not use pytest. I think a better fix here is to rework the structure of testing/_utils.py and move assert_eq to a separate file that doesn't require pytest (or move the bits of testing/_utils.py that require pytest into another location).

I have moved assert_eq into cudf/testing/testing.py (which doesn't need pytest), this aligns with the other assert_XXX_equal functions that were already there. cc2fbf7 then migrates all of the test files to use this new import location.

[bdice]

This is a better solution, I think. The one change worth noting is that cudf.testing.assert_eq is now a public API. (It’s not in the private _utils.py.) Do we need to add it to the API docs?

[wence-]

This is a better solution, I think. The one change worth noting is that cudf.testing.assert_eq is now a public API. (It’s not in the private _utils.py.) Do we need to add it to the API docs?

I have added (and formatted/extended the docstring).

[bdice]

Thanks @wence-!

[wence-]

/merge

[vyasr]

We moved assert_eq it to _utils to avoid it appearing public because other libraries (like cugraph) were relying on its behavior in ways that we didn't want to guarantee stability for (see e.g. rapidsai/cugraph#1693). Do we think that has changed? Is it an API that we actually want to support other libraries using?

If so, we may still want to consider using a different module so that cudf.testing remains a reasonable mirror of pandas.testing instead of including an API for which pandas has no counterpart.

(this isn't important enough to me to be worth blocking this PR though)

[bdice]

We moved assert_eq it to _utils to avoid it appearing public because other libraries (like cugraph) were relying on its behavior in ways that we didn't want to guarantee stability for (see e.g. rapidsai/cugraph#1693). Do we think that has changed? Is it an API that we actually want to support other libraries using?

I did not recall that history, thanks for the reminder.

I am slightly in favor of having a function like assert_eq as a public API. It seems like a crucial utility for libraries consuming cudf, especially when writing tests. I do not want for libraries like cuml or cugraph to have to reinvent such functionality just to be able to test their own libraries when consuming/producing cudf DataFrame or Series objects. Perhaps there is some middle ground where we have some public testing utilities in a module like cudf.testing.utils (to help separate it from being an exact mirror of pandas.testing) and some private testing utilities in cudf.testing._utils (meant only for cudf's own tests)? I don't know. This PR feels like it is moving in the right general direction so I'm happy to merge it in the current state.

[vyasr]

The question is what APIs other libraries need. There are already APIs for comparing DataFrame and Series objects (like assert_frame_equal), and we have to_pandas methods to generate pandas objects for comparison. assert_eq is really an internal convenience because we have historically been inconsistent about argument orders and what types are fed in and many APIs only partially supported proper metadata so things like dtypes could get lost. assert_eq will even do things like trying to automatically handle numpy arrays. I'd prefer not to export that baggage.

[wence-]

We moved assert_eq it to _utils to avoid it appearing public because other libraries (like cugraph) were relying on its behavior in ways that we didn't want to guarantee stability for (see e.g. rapidsai/cugraph#1693). Do we think that has changed? Is it an API that we actually want to support other libraries using?

Turns out this wasn't enough of a deterrence:

• Update cuDF's assert_eq import dask-cuda#1353
• Update cuDF's assert_eq import ucx-py#1050
• Update cuDF's assert_eq import ucxx#239

[bdice]

If we want other libraries to adopt a certain pattern (like not using assert_eq), we should first adopt that pattern in cudf’s own tests. Many other libraries learn how to write tests by reading cudf’s tests, so we might as well make this a public utility.

[vyasr]

There's nothing wrong with having utilities that are only usable within cudf's tests and not elsewhere. That's not happening here in large part because the utility is actually part of the cudf package. If we instead implemented in conftest or a utils module within the tests directory itself, both of which are fairly common patterns, then that would fit the expectations of external users looking at cudf's tests for guidance.