SNOW-2359402 Enabled autoswitching on some unsupported args

[sfc-gh-jenzhang]

1. Which Jira issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

   Fixes SNOW-2359402

2. Fill out the following pre-review checklist:

   • I am adding a new automated test(s) to verify correctness of my new code
     • If this test skips Local Testing mode, I'm requesting review from @snowflakedb/local-testing
   • I am adding new logging messages
   • I am adding a new telemetry message
   • I am adding new credentials
   • I am adding a new dependency
   • If this is a new feature/behavior, I'm adding the Local Testing parity changes.
   • I acknowledge that I have ensured my changes to be thread-safe. Follow the link for more information: Thread-safe Developer Guidelines
   • If adding any arguments to public Snowpark APIs or creating new public Snowpark APIs, I acknowledge that I have ensured my changes include AST support. Follow the link for more information: AST Support Guidelines

3. Please describe how your code solves the related issue.

[sfc-gh-joshi]

Discussed with @sfc-gh-jenzhang offline about the added axis=1 argument to sort_columns_by_row_values, even though it's technically redundant because the QC method always operates on axis=1. This is necessary because when stay_cost is computed, the decorator compares against the frontend method's arguments to return COST_IMPOSSIBLE, but when the wrapped QC method is called, the decorated examines the arguments of the QC method instead.

Alternative options to enable switching for this case were:

• Don't add a QC argument, and use lambda args: args.get("axis", 1) == 1 for the condition instead of just ("axis", 1) -- works but is harder to understand
• Keep unsupported condition as ("axis", 1) but don't add axis=1 argument: will trigger an auto-switch, but when switching is disabled will not trigger the decorator's error message code path because the QC method lacks an axis parameter
• Set unsupported condition to be unconditional (e.g. lambda args: True): will trigger auto-switch even for supported use cases

We decided just adding a docstring comment explaining the addition of the parameter would be better.

[sfc-gh-nkrishna]

Left one question, but approving to unblock

[sfc-gh-nkrishna]

Why remove index?

[sfc-gh-jenzhang]

This is redundant because the frontend layer already parses "index" to 1.

[graphite-app]

The condition logic has a potential issue when handling non-string, non-iterable values for index. If index is a scalar value like an integer, the expression all([isinstance(v, str) for v in args.get("index")]) will attempt to iterate over a non-iterable object, causing a TypeError.

Consider restructuring this condition to first check if the value is iterable before attempting to iterate over it, or use a try/except block to handle this case. A safer approach might be:

lambda args: (
    args.get("index") is not None
    and not isinstance(args.get("index"), str)
    and (
        not hasattr(args.get("index"), "__iter__") 
        or not all(isinstance(v, str) for v in args.get("index"))
    )
    and None not in (args.get("index") if hasattr(args.get("index"), "__iter__") else [args.get("index")])
)

The same issue appears in the similar conditions for columns and values parameters.

Suggested change

	lambda args: (
	args.get("index") is not None
	and (
	not isinstance(args.get("index"), str)
	and not all([isinstance(v, str) for v in args.get("index")])
	and None not in args.get("index")
	)
	),
	lambda args: (
	args.get("index") is not None
	and not isinstance(args.get("index"), str)
	and (
	not hasattr(args.get("index"), "__iter__")
	or not all(isinstance(v, str) for v in args.get("index"))
	)
	and None not in (
	args.get("index") if hasattr(args.get("index"), "__iter__") else [args.get("index")]
	)
	),

Spotted by Graphite Agent



Is this helpful? React 👍 or 👎 to let us know.