@tseaver I am not familiar with the review process in these repositories. Should I push a new commit with the suggested changes, and rebase at the end? or should I rebase now?

Thanks for reviewing!

@guillermo-carrasco

I am not familiar with the review process in these repositories. Should I push a new commit with the suggested changes, and rebase at the end? or should I rebase now?

Push a new commit on the same branch. I will review those changes. We iterate until I hit the "approve" button for the review, and then I hit the merge button.

Occasionally I might ask for a rebase against the currentmaster (e.g., if the CI machinery gets confused about what has changed), but I wouldn't ask that you squash commits in such a rebase: instead, when I merge, I will squash all commits for the PR into one.

Thanks for reviewing!

Thank you for the patch!

@tseaver thanks for the clarification. I just amended the suggested changes.

@shollyman,@tswast I'm fine with the patch as it stands now. Please comment on suitability from the DPE side.

This new commit makes it easier to use dictionaries with the%%bigquery magic. Now one can pass in a previously built dictionary, instead of having to write it down in the magic line:

In [1]: params = {"min_word_count": 200, "corpus": "romeoandjuliet"}In [2]: %%bigquery df --params $params    ...:     ...: SELECT word, word_count    ...: FROM `bigquery-public-data.samples.shakespeare`    ...: WHERE corpus = "{corpus}"    ...: AND word_count >= {min_word_count}    ...: ORDER BY word_count DESC;    ...:

@tswast please disregard my last comment. I have folowed your suggestion and usedto_query_parameters from BigQuery's built-in query parameter handling. Now the parametrizing in notebooks works in line with the SDK:

In [1]: params = {"min_word_count": 200, "corpus": "romeoandjuliet"}In [2]: %%bigquery df --params $params   ...:    ...: SELECT word, word_count   ...: FROM `bigquery-public-data.samples.shakespeare`   ...: WHERE corpus = @corpus   ...: AND word_count >= @min_word_count   ...: ORDER BY word_count DESC;

Sorry for closing, it was a mistake. Thanks for the review@alixhami. I'll implement the suggested changes.

@alixhami I have implemented the following changes:

• Added a test for expanding a dictionary
• Importing module instead of function
• Fixeddf typo (also in the other test)

I am not sure that changing the docstring to reflect the dict approach is what we want though. It is still a JSON string what is sent in the cell magic. It's just that it is also possible to expand a dictionaryinstead of writing the JSON string. But at the end, it is a JSON string what is treated in_cell_magic. Do you still think that I should change the documentation? I have added usage examples for both cases.

Regarding contributions: I totally understand, and I appreciate you're taking the time to review this. Query parametrization is something we use a lot, and we have a heavy notebook-drived development, so this would be a great addition for us.

Thanks again!

Thanks@guillermo-carrasco!

@tswast - what is your preference on how this parameter should be documented?

I've slightly changed the docstring now. I had to remove the dictionary reference example from the documentation. The reason is that since$ is ipython specific syntax, sphinx is failing on building the documentation. I couldn't find a way to overcome this. I'm open to suggestions, thanks!

@guillermo-carrasco I'm looking into the docs build issue now. I think that example is very helpful, so I'm going to see if there's a way to include it without throwing errors.

@alixhami PTAL one final pass.

Ok I pushed some commits, which make the following updates:

• Remove python syntax highlighting (because it isn't actually python anyway), so the dict params example could be added back in
• Fix lint and coverage
• Update the documentation for theparams parameter to be more detailed and point to the relevant examples. The trade off I made here is favoring clarity at the expense of being repetitive. I ran the docstring updates by@tswast offline.

amazing, thanks a lot for all the help and reviews :)