fix: Pass appropriate kwargs to dummy data generators

[rebkwok]

This lets dataset definitions run with the cli use the new additional_population_constraints

[cloudflare-workers-and-pages]

Deploying databuilder-docs with    Cloudflare Pages

Latest commit:	e83d013
Status:	✅  Deploy successful!
Preview URL:	https://6ba14e0f.databuilder.pages.dev
Branch Preview URL:	https://dummy-data-kwargs.databuilder.pages.dev

View logs

[evansd]

Thanks for catching this. I'd like to have a think about what kind of testing approach would have stopped this making it into prod, but that's a wider question.

Now that we're returning kwargs I think there might be a slightly more straightforward factoring here, along the lines of:

def get_dummy_data_generator(variable_definitions, dummy_data_config):
    if dummy_data_config.legacy:
        return DummyDataGenerator(variable_definitions, ...)
    else:
        return NextGenDummyDataGenerator(variable_definitions, ...)

Returning a reference to the class and the constructor kwargs feels like unnecessary indirection at this point.

[rebkwok]

Thanks for catching this. I'd like to have a think about what kind of testing approach would have stopped this making it into prod, but that's a wider question.

Now that we're returning kwargs I think there might be a slightly more straightforward factoring here, along the lines of:

def get_dummy_data_generator(variable_definitions, dummy_data_config):
    if dummy_data_config.legacy:
        return DummyDataGenerator(variable_definitions, ...)
    else:
        return NextGenDummyDataGenerator(variable_definitions, ...)

Returning a reference to the class and the constructor kwargs feels like unnecessary indirection at this point.

Yeah, I'd thought about doing it that way, happy to change it