Skip to content

refactor: Emit FutureWarning on nullable primary key #41

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 1 commit into from
Jun 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions dataframely/_deprecation.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,14 @@ def warn_nullable_default_change() -> None:
FutureWarning,
stacklevel=4,
)


@skip_if(env="DATAFRAMELY_NO_FUTURE_WARNINGS")
def warn_no_nullable_primary_keys() -> None:
warnings.warn(
"Nullable primary keys are not supported. "
"Setting `nullable=True` on a primary key column is ignored "
"and will raise an error in a future release.",
FutureWarning,
stacklevel=4,
)
9 changes: 8 additions & 1 deletion dataframely/columns/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@
import polars as pl

from dataframely._compat import pa, sa, sa_TypeEngine
from dataframely._deprecation import warn_nullable_default_change
from dataframely._deprecation import (
warn_no_nullable_primary_keys,
warn_nullable_default_change,
)
from dataframely._polars import PolarsDataType
from dataframely.random import Generator

Expand Down Expand Up @@ -67,6 +70,10 @@ def __init__(
internally sets the alias to the column's name in the parent schema.
metadata: A dictionary of metadata to attach to the column.
"""

if nullable and primary_key:
warn_no_nullable_primary_keys()

if nullable is None:
warn_nullable_default_change()
nullable = True
Expand Down
45 changes: 40 additions & 5 deletions tests/test_deprecation.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,62 @@
# SPDX-License-Identifier: BSD-3-Clause

import warnings
from collections.abc import Callable

import pytest

import dataframely as dy

# --------------------- Nullability default change ------------------------------#

def test_column_constructor_warns_about_nullable(

def deprecated_default_nullable() -> None:
"""This function causes a FutureWarning because no value is specified for the
`nullable` argument to the Column constructor."""
dy.Integer()


def test_warning_deprecated_default_nullable(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setenv("DATAFRAMELY_NO_FUTURE_WARNINGS", "")
with pytest.warns(
FutureWarning, match="The 'nullable' argument was not explicitly set"
):
dy.Integer()
deprecated_default_nullable()


# ------------------------- Nullable primary key ---------------------------------#


def deprecated_nullable_primary_key() -> None:
"""This function causes a FutureWarning because both `nullable` and `primary_key`
are set to `True` in the Column constructor."""
dy.Integer(primary_key=True, nullable=True)


def test_warning_deprecated_nullable_primary_key(
monkeypatch: pytest.MonkeyPatch,
) -> None:
monkeypatch.setenv("DATAFRAMELY_NO_FUTURE_WARNINGS", "")
with pytest.warns(FutureWarning, match="Nullable primary keys are not supported"):
deprecated_nullable_primary_key()


# ------------------------- Common ---------------------------------#


@pytest.mark.parametrize(
"deprecated_behavior",
[deprecated_default_nullable, deprecated_nullable_primary_key],
)
@pytest.mark.parametrize("env_var", ["1", "True", "true"])
def test_future_warning_skip(monkeypatch: pytest.MonkeyPatch, env_var: str) -> None:
def test_future_warning_skip(
monkeypatch: pytest.MonkeyPatch, env_var: str, deprecated_behavior: Callable
) -> None:
"""FutureWarnings should be avoidable by setting an environment variable."""
monkeypatch.setenv("DATAFRAMELY_NO_FUTURE_WARNINGS", env_var)

# Elevates FutureWarning to an exception
with warnings.catch_warnings():
warnings.simplefilter("error", FutureWarning)
dy.Integer()
deprecated_behavior()
Loading