Improve querying

[olejandro]

This PR allows other fields (in addition to process and commodity) to be passed as lists to query. It also allows column-wise control of which dataframe entries should be exploded if a comma is found.

[siddharth-krishna]

Thanks, Olex, this looks good.

I'm wondering if we need to remove the old match_uc_wildcards test though? It's common practice to test a function by comparing its results to a slower-but-simpler implementation, and I think we don't have unit tests that test the new code on a series of small examples, do we? So why not leave it in.

(Ideally, eventually, we would have unit tests for more transforms using small examples.)

[siddharth-krishna]

Wow, this PR also improves runtime by around 50%! Do you have any idea what is responsible for the speedup? 🤩

[olejandro]

Wow, this PR also improves runtime by around 50%! Do you have any idea what is responsible for the speedup? 🤩

Good question! TIMES-NZ runs are responsible for the reported wild improment. It's runtimes have varied a lot before. As far as I can see from the log, the time it takes to run convert_to_string transform went down significantly with the changes in this PR. Let me check what changed for it...

[olejandro]

I'm wondering if we need to remove the old match_uc_wildcards test though? It's common practice to test a function by comparing its results to a slower-but-simpler implementation, and I think we don't have unit tests that test the new code on a series of small examples, do we? So why not leave it in.

The changes in this PR brake the test, because some inputs change. Since we have had the faster version running for a while now, without ever having any issues with it, I thought it was okay just to delete the test. I could try to fix it instead, if you think it is worth keeping it?

[olejandro]

Wow, this PR also improves runtime by around 50%! Do you have any idea what is responsible for the speedup? 🤩

Good question! TIMES-NZ runs are responsible for the reported wild improment. It's runtimes have varied a lot before. As far as I can see from the log, the time it takes to run convert_to_string transform went down significantly with the changes in this PR. Let me check what changed for it...

I've timed it: convert_to_string take less time on the mig table now. It is probably because the table has fewer rows, since we keep previously exploded entries as lists for querying.

[siddharth-krishna]

Thanks for looking into it! And no, not worth spending too much time on the test. It might be easier to write unit tests once we've converted many transforms into methods of the TimesModel class, perhaps.