I wish Python had Rust enums. Any recommendations on the cleanest way to model this type as

None | ExportPythonTool

for a rule return type?

@dataclass(frozen=True)
class ExportPythonTool:
    resolve_name: str
    pex_request: PexRequest

You can't return Optional[ExportPythonTool] ?

I think the rule graph engine isn't equipped for that 😕

Ahhh, okay

yeah, unfortunately 😕 So then I could add

MaybeExportPythonTool

, but now we have two class definitions and imports - ew

In the end, you need a concrete type because what else are you gonna request?

Feels like a NullObject pattern

THat's how we normally solve this, e.g.

FmtResult.skip()

I suppose we could try and special-case optional types into the rule engine?

I guess I could keep

resolve_name: str

, and make it be

pex_request: PexRequest | None

! It's cheap to compute the resolve name Before I was thinking of making both fields

| None

, which is icky

Why not simply add a new

skip

field to

ExportPythonTool.skip

?

because you wouldn't have something to set

PexRequest()

too - there is no Nully version of it

No?

async def black_export(
    _: BlackExportSentinel, black: Black, python_setup: PythonSetup
) -> ExportPythonTool:
    constraints = await _black_interpreter_constraints(black, python_setup)
    return ExportPythonTool(
        resolve_name=black.options_scope,
        pex_request=black.to_pex_request(interpreter_constraints=constraints),
        skip=not black.export,
    )

Ah, it's crucial that we early exit -

await _black_interpreter_constraints

has a performance hit

if interpreter constraints are expensive, they could be set to None when export is false.

this is the rule now:

@rule
async def black_export(
    _: BlackExportSentinel, black: Black, python_setup: PythonSetup
) -> ExportPythonTool:
    if not black.export:
        return ExportPythonTool(resolve_name=black.options_scope, pex_request=None)
    constraints = await _black_interpreter_constraints(black, python_setup)
    return ExportPythonTool(
        resolve_name=black.options_scope,
        pex_request=black.to_pex_request(interpreter_constraints=constraints),
    )

the

to_pex_request

doesn’t seem expensive (from what I can read)

fyi https://github.com/pantsbuild/pants/pull/15509