[MNT] Deprecate LogCpTransformer in favour of LogTransformer with C parameter by adityaanikam · Pull Request #970 · feature-engine/feature_engine

adityaanikam · 2026-07-24T10:48:03Z

Closes #957

Summary

Merges LogCpTransformer into LogTransformer as approved in the issue
discussion. LogTransformer gains a C parameter ("auto" / int / float /
dict) mirroring LogCpTransformer's exact semantics, defaulting to C=0 so
existing LogTransformer() usage is unchanged. LogCpTransformer becomes a
thin subclass defaulting to C="auto", noted as being consolidated into
LogTransformer in favor of LogTransformer(C=...).

Backward compatibility

LogTransformer()'s behavior and error message/timing are unchanged:
fit-time validation still fires exactly when C resolves to 0 (the new
default), with the original message.
LogCpTransformer's numerical behavior and C_ computation are unchanged.

Things worth flagging

LogCpTransformer's base= error message lost its dynamic
Got {base} instead. suffix — it now uses LogTransformer's original
plain message, since that's the class whose wording was promised
unchanged. Updated the corresponding test to match.
LogCpTransformer gets its own _more_tags()/__sklearn_tags__()
overrides resetting to the un-xfailed tags, so it doesn't silently
inherit LogTransformer's zero-value xfail exemptions that it never
needed (its "auto" default always finds a valid shift).
There's no existing convention in this codebase for deprecating a class
(searched — no precedent), so I kept the notice to a plain docstring
.. note:: rather than inventing a formal .. deprecated:: version
directive. I also didn't touch docs/user_guide/transformation/ LogCpTransformer.rst, which still describes it as fully independent —
happy to update that in this PR or a follow-up, whichever you'd prefer.

The 0-handling-without-a-declared-constant question from the issue is left
for a follow-up, per the discussion.

Testing

pytest tests/test_transformation/ — 117 passed, including a new regression
test proving the C=0 default preserves the original fail-fast contract.

…ndentation

solegalli

Hi @adityaanikam

This is looking really good! Thank you so much for the first draft.

I left a few comments, mostly to add future warnings about the deprecation, and to add the tests for C in the logtransform tests.

The last bit that needs updating to have this one ready is the user guide. Inside the docs folder, useguide/transformers, we need to add the demo of the use of C to Logtransfor.rst. You could probably re-adapt the examples that we have for logcp.

Thanks a lot!

solegalli · 2026-07-26T05:25:12Z

+        The constant C to add to the variable before the logarithm, i.e., log(x + C).
+
+        - If 0 (the default), no constant is added and the variable must be
+          strictly positive, matching the transformer's original behavior.


Suggested change

strictly positive, matching the transformer's original behavior.

strictly positive.

solegalli · 2026-07-26T05:26:06Z

-    The LogTransformer() only works with positive values. If the variable
-    contains a zero or a negative value the transformer will return an error.
+    By default, C=0, so LogTransformer() only works with positive values and
+    behaves exactly as it always has: if a variable contains a zero or a negative


Suggested change

behaves exactly as it always has: if a variable contains a zero or a negative

If a variable contains a zero or a negative

solegalli · 2026-07-26T05:26:18Z


-    The LogTransformer() only works with positive values. If the variable
-    contains a zero or a negative value the transformer will return an error.
+    By default, C=0, so LogTransformer() only works with positive values and


Suggested change

By default, C=0, so LogTransformer() only works with positive values and

By default, C=0, so LogTransformer() only works with positive values.

solegalli · 2026-07-26T05:26:46Z

-    contains a zero or a negative value the transformer will return an error.
+    By default, C=0, so LogTransformer() only works with positive values and
+    behaves exactly as it always has: if a variable contains a zero or a negative
+    value, the transformer raises an error.


Could we add a note saying that C will be changed from 0 to "auto" in version 2.1.0? I think this is the easiest way to avoid the error on negative values that we were discussing earlier.

solegalli · 2026-07-26T05:29:09Z

    {variables_}

+    C_:
+        The constant C added to each variable. Equal to C, unless C = "auto", in


Suggested change

The constant C added to each variable. Equal to C, unless C = "auto", in

The constant C added to each variable. Equal to `C`, unless `C = "auto"`, in

solegalli · 2026-07-26T05:31:08Z

-        can be applied on the selected variables, i.e., it checks that the variables
-        are positive.
+        Selects the numerical variables and, when C=0 (the default), determines
+        whether the logarithm can be applied on the selected variables, i.e., it


I think we should remove lines 157-159.

solegalli · 2026-07-26T05:36:12Z

-        The constant C to add to each variable. If C = "auto" a dictionary with
-        C = abs(min(variable)) + 1. For strictly positive variables, C = 0.
+    .. note::
+        `LogCpTransformer` is being consolidated into `LogTransformer`. New


Could we add the version changes and future deprecations in this note? Something like:

LogCpTransformer is consolidated into LogTransformer in version 2.0.0 and will be deprecated in version 2.1.0. New code....

solegalli · 2026-07-26T05:36:42Z

-class LogCpTransformer(BaseNumericalTransformer, FitFromDictMixin):
+class LogCpTransformer(LogTransformer):
    """
    LogCpTransformer() applies the transformation log(x + C), where x is the


Suggested change

LogCpTransformer() applies the transformation log(x + C), where x is the

#TODO: remove in version 2.1.0

LogCpTransformer() applies the transformation log(x + C), where x is the

solegalli · 2026-07-26T05:39:29Z

+        super().__init__(
+            variables=variables, return_empty=return_empty, base=base, C=C
        )



Could we add a future warning in the init saying that LogCP... was deprecated in favour of LogTransformer in version 2.0.0 and will be removed in version 2.1.0. Use LogTransformer() instead? So every time someone uses this class, they are alerted that it will disappear in the next version?

solegalli · 2026-07-26T05:45:58Z

    # test transform output
    pd.testing.assert_frame_equal(X, df_vartypes)
+
+


We should bring the tests for C, from test_logcp to this file as well, so we know it works as intended.

- Emit a FutureWarning on LogCpTransformer instantiation, announcing its removal in version 2.1.0 in favour of LogTransformer(C=auto). - Align the LogCpTransformer docstring note with that warning: it is deprecated in 2.0.0 and removed in 2.1.0, not deprecated in 2.1.0. - Explain why `variables` is ignored when C is a dictionary (the variables are taken from the dictionary keys) instead of restating that C accepts dictionaries, which the parameter list above already documents. - Mark the pending removal with a TODO above the class statement so it stays a developer note rather than rendered documentation text. - Note in LogTransformer's docstring that C will default to auto in 2.1.0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Port the C-parameter tests from test_logcp_transformer.py to test_log_transformer.py, covering parameter validation, C=auto/dict/int at fit time, the transform-time positivity check, both logarithm bases and inverse_transform. Uses a `df_c` fixture to avoid colliding with the existing conftest fixtures. Document the new parameter in the user guide: replace the note stating that non-positive values always raise and pointing to the now-deprecated LogCpTransformer, and add a section demonstrating C=auto on the diabetes dataset. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

adityaanikam · 2026-07-26T13:39:29Z

Hi @solegalli, just wanted to flag something separate from the code review. CI has failed in the same way on three different runs of this branch now. Most recently, test_feature_engine_py313 and test_feature_engine_py314 failed on 7eae67fd, each with the same result:

51 failed, 1969 passed

All 51 failures seem to come from fetch_california_housing() failing with:

sklearn.datasets._base._fetch_remote → urlretrieve → urllib.error.HTTPError: HTTP Error 403: Forbidden

That accounts for 48 failures in test_wrappers/test_sklearn_wrapper.py, 2 in test_discretisation, and 1 in test_selection/test_mrmr.py. They’re all tests that fetch the California housing dataset, and none of the failures are in test_transformation.

The Python version that fails also seems to vary between runs py310 on one, py314 on another, and py313 + py314 most recently so it looks more like a flaky download/CI issue than something caused by this change.

I had a look at the dataset URL as well. It redirects to a presigned S3 URL with X-Amz-Expires=10, so the signature only has a 10 second window. I wonder if a runner occasionally takes too long to follow the redirect and ends up hitting an expired URL. The download works consistently for me locally.

Since fetch_california_housing() caches the dataset under ~/scikit_learn_data after the first successful download, caching that directory in CircleCI might avoid this altogether.

Happy to look into that if it would be useful, but wanted to flag it first in case you’re already aware of it or have a preferred way of handling it.

adityaanikam added 8 commits July 24, 2026 16:06

Merge LogCpTransformer into LogTransformer with C parameter

2dcb068

Merge LogCpTransformer into LogTransformer with C parameter

dc3cc3c

Fix style: trailing newline, blank line spacing, and note directive i…

07aca63

…ndentation

Fix style: trailing newline, blank line spacing, and note directive i…

e5024f8

…ndentation

Fix style: trailing newline, blank line spacing, and note directive i…

17ca81a

…ndentation

Fix E302: add missing blank line before new test

b34011e

Fix W293: remove trailing whitespace from blank line

fd17522

Normalize log.py line endings to LF, matching repo convention

bf16b34

solegalli reviewed Jul 26, 2026

View reviewed changes

solegalli changed the title ~~Merge LogCpTransformer into LogTransformer with C parameter~~ [MNT] Deprecate LogCpTransformer in favour of LogTransformer with C parameter Jul 26, 2026

adityaanikam and others added 4 commits July 26, 2026 12:44

Add the offending value to the base validation error message

2b5c9b9

Include dictionaries in the C validation error message

7eae67f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[MNT] Deprecate LogCpTransformer in favour of LogTransformer with C parameter#970

[MNT] Deprecate LogCpTransformer in favour of LogTransformer with C parameter#970
adityaanikam wants to merge 12 commits into
feature-engine:mainfrom
adityaanikam:merge-log-transformers-957

adityaanikam commented Jul 24, 2026

Uh oh!

solegalli left a comment

Uh oh!

solegalli Jul 26, 2026

Uh oh!

solegalli Jul 26, 2026

Uh oh!

solegalli Jul 26, 2026

Uh oh!

solegalli Jul 26, 2026

Uh oh!

solegalli Jul 26, 2026

Uh oh!

solegalli Jul 26, 2026

Uh oh!

solegalli Jul 26, 2026

Uh oh!

solegalli Jul 26, 2026

Uh oh!

solegalli Jul 26, 2026

Uh oh!

solegalli Jul 26, 2026

Uh oh!

adityaanikam commented Jul 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	strictly positive, matching the transformer's original behavior.
	strictly positive.

	behaves exactly as it always has: if a variable contains a zero or a negative
	If a variable contains a zero or a negative

	By default, C=0, so LogTransformer() only works with positive values and
	By default, C=0, so LogTransformer() only works with positive values.

	The constant C added to each variable. Equal to C, unless C = "auto", in
	The constant C added to each variable. Equal to `C`, unless `C = "auto"`, in

	LogCpTransformer() applies the transformation log(x + C), where x is the
	#TODO: remove in version 2.1.0
	LogCpTransformer() applies the transformation log(x + C), where x is the

		# test transform output
		pd.testing.assert_frame_equal(X, df_vartypes)

Uh oh!

Conversation

adityaanikam commented Jul 24, 2026

Summary

Backward compatibility

Things worth flagging

Testing

Uh oh!

solegalli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adityaanikam commented Jul 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants