Skip to content

Commit

Permalink
Address PR comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
vyasr committed Feb 15, 2022
1 parent 55e81ec commit cc72147
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 50 deletions.
4 changes: 2 additions & 2 deletions python/cudf/cudf/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import rmm # noqa: F401

_CURRENT_DIRECTORY = os.path.dirname(os.path.realpath(__file__))
_CURRENT_DIRECTORY = str(pathlib.Path(__file__).resolve().parent)


@pytest.fixture(scope="session")
Expand All @@ -18,7 +18,7 @@ def datadir():
# To set and remove the NO_EXTERNAL_ONLY_APIS environment variable we must use
# the sessionstart and sessionfinish hooks rather than a simple autouse,
# session-scope fixture because we need to set these variable before collection
# occurs because the envrionment variable will be checked as soon as cudf is
# occurs because the environment variable will be checked as soon as cudf is
# imported anywhere.
def pytest_sessionstart(session):
"""
Expand Down
91 changes: 43 additions & 48 deletions python/cudf/cudf/utils/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,61 +42,56 @@
# The test root is set by pytest to support situations where tests are run from
# a source tree on a built version of cudf.
NO_EXTERNAL_ONLY_APIS = os.getenv("NO_EXTERNAL_ONLY_APIS")
_CUDF_TEST_ROOT = os.getenv("_CUDF_TEST_ROOT")

if NO_EXTERNAL_ONLY_APIS in ("True", "1", "TRUE"):
_cudf_root = cudf.__file__.replace("/__init__.py", "")
# If the environment variable for the test root is not set, we default to
# using the path relative to the cudf root directory.
_tests_root = _CUDF_TEST_ROOT or os.path.join(_cudf_root, "tests")

def _external_only_api(func, alternative=""):
"""Decorator to indicate that a function should not be used internally.
cudf contains many APIs that exist for pandas compatibility but are
intrinsically inefficient. For some of these cudf has internal
equivalents that are much faster. Usage of the slow public APIs inside
our implementation can lead to unnecessary performance bottlenecks.
Applying this decorator to such functions and then setting the
environment variable NO_EXTERNAL_ONLY_APIS will cause such functions to
raise exceptions if they are called from anywhere inside cudf, making
it easy to identify and excise such usage.
"""

# If the first arg is a string then an alternative function to use in
# place of this API was provided, so we pass that to a subsequent call.
# It would be cleaner to implement this pattern by using a class
# decorator with a factory method, but there is no way to generically
# wrap docstrings on a class (we would need the docstring to be on the
# class itself, not instances, because that's what `help` looks at) and
# there is also no way to make mypy happy with that approach.
if isinstance(func, str):
return lambda actual_func: _external_only_api(actual_func, func)

@functools.wraps(func)
def wrapper(*args, **kwargs):
# Check the immediately preceding frame to see if it's in cudf.
frame, lineno = next(traceback.walk_stack(None))
fn = frame.f_code.co_filename
if _cudf_root in fn and _tests_root not in fn:
raise RuntimeError(
f"External-only API called in {fn} at line {lineno}. "
f"{alternative}"
)
return func(*args, **kwargs)

return wrapper
_cudf_root = os.path.dirname(cudf.__file__)
# If the environment variable for the test root is not set, we default to
# using the path relative to the cudf root directory.
_tests_root = os.getenv("_CUDF_TEST_ROOT") or os.path.join(_cudf_root, "tests")


def _external_only_api(func, alternative=""):
"""Decorator to indicate that a function should not be used internally.
else:
cudf contains many APIs that exist for pandas compatibility but are
intrinsically inefficient. For some of these cudf has internal
equivalents that are much faster. Usage of the slow public APIs inside
our implementation can lead to unnecessary performance bottlenecks.
Applying this decorator to such functions and setting the environment
variable NO_EXTERNAL_ONLY_APIS will cause such functions to raise
exceptions if they are called from anywhere inside cudf, making it easy
to identify and excise such usage.
def _external_only_api(func, alternative=""):
"""The default implementation is a no-op."""
if isinstance(func, str):
return lambda actual_func: _external_only_api(actual_func, func)
The `alternative` should be a complete phrase or sentence since it will
be used verbatim in error messages.
"""

# If the first arg is a string then an alternative function to use in
# place of this API was provided, so we pass that to a subsequent call.
# It would be cleaner to implement this pattern by using a class
# decorator with a factory method, but there is no way to generically
# wrap docstrings on a class (we would need the docstring to be on the
# class itself, not instances, because that's what `help` looks at) and
# there is also no way to make mypy happy with that approach.
if isinstance(func, str):
return lambda actual_func: _external_only_api(actual_func, func)

if not NO_EXTERNAL_ONLY_APIS:
return func

@functools.wraps(func)
def wrapper(*args, **kwargs):
# Check the immediately preceding frame to see if it's in cudf.
frame, lineno = next(traceback.walk_stack(None))
fn = frame.f_code.co_filename
if _cudf_root in fn and _tests_root not in fn:
raise RuntimeError(
f"External-only API called in {fn} at line {lineno}. "
f"{alternative}"
)
return func(*args, **kwargs)

return wrapper


def scalar_broadcast_to(scalar, size, dtype=None):

Expand Down

0 comments on commit cc72147

Please sign in to comment.