Make ascii_tree's optional df argument usable, and stop it mutating the caller's frame

[ppcvote]

• resolves ascii_tree() raises ValueError whenever its optional df argument is supplied #1224

What was wrong

ascii_tree(dt, df) raised ValueError for every non-None df, so the optional second parameter in its own signature could never be used.

helpers.py:380 tested the sentinel with ==:

def ascii_tree(dt: DecisionTable, df: pd.DataFrame | None = None) -> str:
    if df == None:
        df = decision_table_to_longform_df(dt)

On a DataFrame == is elementwise, so it returns a same-shaped boolean frame and the enclosing if calls DataFrame.__bool__, which pandas raises from by design. The default path survived only because None == None is a plain scalar True. Every caller in the repository and both README examples omit df, which is why nothing ever exercised it. base.py:706 re-exports the function, so both public routes failed the same way.

What changed

Two lines.

if df == None: becomes if df is None:. The parameter is documented as pd.DataFrame | None and the sentinel is identity, so is is the right test rather than isinstance.

The "row" column is no longer dropped in place. This is the part I would most like a second opinion on, because it is not in the issue.

The in-place drop was harmless only while df could not be supplied: the frame always belonged to this function. Making the argument work makes the mutation reachable. A caller who passes a frame carrying a "row" column silently loses that column from their own object. Measured on unmodified code, with the is None fix applied so the call could proceed at all:

caller df columns before: ['row', 'example:W:1.0.0', 'example:H:1.0.0', 'basic:YN:1.0.0']
caller df columns after : ['example:W:1.0.0', 'example:H:1.0.0', 'basic:YN:1.0.0']

df = df.drop(columns="row") keeps the column out of the tree while leaving the caller's object alone. If you would rather keep the in-place drop and document the mutation instead, that is a reasonable call and I will split it out.

Tests

ascii_tree had no test coverage, so this adds src/test/decision_tables/test_helpers.py with five tests:

test	on unmodified main
test_df_omitted	passes
test_df_supplied	fails
test_df_supplied_via_base	fails
test_caller_frame_is_not_modified	fails
test_row_column_is_excluded_from_the_tree	fails

The first is deliberate: it covers the path every current caller takes and passes either way, so it demonstrates the fixture and the suite is not vacuous. The other four fail before and pass after, confirmed by stashing the change and re-running.

src/test has the same single pre-existing failure before and after this branch, test_main.py::MyTestCase::test_expected_routers, which is unrelated.

black --check is clean on the new test file. I left helpers.py formatting alone: it does not satisfy black on main either, the diff black wants is in the HTML block well away from this change, and the psf/black step in python-app.yml is commented out, so reformatting it here would be unrelated churn.

[sei-vsarvepalli]

Thanks for this patch.