Skip to content

[More Language Features] Adding * Operators #258

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Jan 19, 2023
Merged

[More Language Features] Adding * Operators #258

merged 17 commits into from
Jan 19, 2023

Conversation

HumphreyYang
Copy link
Member

This PR resolves #257 by adding a section on the use of *.

@HumphreyYang HumphreyYang marked this pull request as draft December 5, 2022 06:17
@github-actions
Copy link

github-actions bot commented Dec 5, 2022

@HumphreyYang
Copy link
Member Author

Hi @mmcky,

Could you have a quick review on this PR to see where I should further improve?

Many thanks in advance!

@HumphreyYang HumphreyYang requested a review from mmcky December 5, 2022 06:35
@mmcky
Copy link
Contributor

mmcky commented Dec 13, 2022

@HumphreyYang I have updated the build infrastructure from the main branch. I would like to use this change as a final test case. Let me know if you spot any issues. Thanks.

@github-actions
Copy link

github-actions bot commented Dec 13, 2022

@github-actions github-actions bot temporarily deployed to pull request December 13, 2022 07:17 Inactive
@HumphreyYang
Copy link
Member Author

@HumphreyYang I have updated the build infrastructure from the main branch. I would like to use this change as a final test case. Let me know if you spot any issues. Thanks.

Many thanks @mmcky, the build runs much faster! I did not see any notable issues in the build.

@github-actions github-actions bot temporarily deployed to pull request December 13, 2022 07:44 Inactive
Copy link
Contributor

@mmcky mmcky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@HumphreyYang thanks for putting this together.

I am not sure that we should talk much about arbitrary functions *args and **kwargs as I don't think this is a great fit for the intended audience of this lecture. What do you think?

Yang, Humphrey (Data61, Clayton) added 2 commits December 21, 2022 00:55
@github-actions github-actions bot temporarily deployed to pull request December 20, 2022 14:10 Inactive
@github-actions github-actions bot temporarily deployed to pull request December 21, 2022 08:21 Inactive
@github-actions github-actions bot temporarily deployed to pull request December 21, 2022 14:01 Inactive
@github-actions github-actions bot temporarily deployed to pull request December 23, 2022 06:05 Inactive
@HumphreyYang
Copy link
Member Author

HumphreyYang commented Dec 23, 2022

Hi @mmcky,

Thanks for your spot-on feedback last time. I have pushed a simplified version of the PR. I made some choices during the process:

  • I have used a conceptually easier example instead of the one based on GBM I wrote before.

  • I also left out an exercise that asks readers to replicate the generate_plots function in the example using args and **kargs. The solution will be something like this:

def generate_plots(*β_ls, idx=0, **kargs):
    label_list = []
    for βs in zip(β_ls[0::2], β_ls[1::2]):
        ax[idx].plot(*generate_data(*βs), **kargs['line_kargs'])
        label_list.append(f'$β0 = {βs[0]}$ / $β1 = {βs[1]}$')
    ax[idx].legend(label_list, **kargs['legend_kargs'])

β_ls = [10, 1,
        20, 2,
        30, 3]

generate_plots(*β_ls, line_kargs=line_kargs, legend_kargs=legend_kargs, idx=0)

β_ls.extend([40, 1])

generate_plots(*β_ls, line_kargs=line_kargs, legend_kargs=legend_kargs, idx=1)

plt.show()

I think this exercise is a good one to help readers to distinguish * ** in the context of function definitions and function calls, but I left out this part because it uses assumed knowledge from many previous lectures and may misdirect readers.

I was curious about what is your opinions on the current version and the compromises I have made.

Many thanks in advance. Have a lovely Christmas and New Year! 🎄

@github-actions github-actions bot temporarily deployed to pull request December 23, 2022 06:28 Inactive
@HumphreyYang HumphreyYang marked this pull request as ready for review January 2, 2023 05:22
@mmcky
Copy link
Contributor

mmcky commented Jan 9, 2023

thanks @HumphreyYang I have taken another look at the simplified version. I am still wondering if this is too much -- it is a good example though. I might be really biased as I tend not to use these operators much at all.

@jstac do you have good example? I imagine the most common quantecon use case is when passing parameters to models?

# Set up the frame and subplots
fig, ax = plt.subplots(2, 1)
plt.subplots_adjust(hspace=0.7)
plt.rcParams["figure.figsize"] = (10, 6)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need line 356? This is usually at the top of each lecture.

@jstac
Copy link
Contributor

jstac commented Jan 10, 2023

Hi all,

I think this is maybe a fraction long, given that we work hard to keep discussion very short, but nicely written and a good example. I recommend merging conditional on addressing the minor comment above.

Nice work @HumphreyYang .

@github-actions github-actions bot temporarily deployed to pull request January 10, 2023 13:22 Inactive
@HumphreyYang HumphreyYang requested a review from mmcky January 10, 2023 23:15
@HumphreyYang
Copy link
Member Author

Hi @mmcky,

Thanks for your reviews. I have addressed the issue @jstac raised. Could you please merge this PR if it looks to you?

Many thanks.

@mmcky
Copy link
Contributor

mmcky commented Jan 19, 2023

thanks @HumphreyYang and @jstac will merge this in now.

@mmcky mmcky merged commit d27d526 into main Jan 19, 2023
@mmcky mmcky deleted the add-splat branch January 19, 2023 00:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[More Language Features] Add Splat Operator, *args, and **kwargs to Lecture 17
3 participants