+
Skip to content

Do not use mutable value for defaults #169

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 1 commit into from
Jul 10, 2025

Conversation

kajinamit
Copy link
Contributor

The default values are generated only one, and using a mutable value as a default causes unintentional shared value updated by different method calls.

The default values are generated only one, and using a mutable value
as a default causes unintentional shared value updated by different
method calls.
@kajinamit kajinamit force-pushed the no-mutable-defaults branch from 8ebc1c1 to 2e05dc4 Compare July 10, 2025 13:21
@@ -221,7 +221,7 @@ class PecanBase(object):

def __init__(self, root, default_renderer='mako',
template_path='templates', hooks=lambda: [],
custom_renderers={}, extra_template_vars={},
Copy link
Member

@ryanpetrello ryanpetrello Jul 10, 2025

Choose a reason for hiding this comment

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

These aren't assigned defaults.

Also, pecan.templating.RendererFactory has the same problem in its constructor.

Copy link
Contributor Author

@kajinamit kajinamit Jul 10, 2025

Choose a reason for hiding this comment

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

RendererFactory translates custom_renderers=None to {}, and also extra_vars=None is translated to {} by ExtraNamespace , which is called by RendererFactory.
I can add the logic to translate None in every layer but I avoided it because it is technically redundant.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see. Yep, that makes sense.

@ryanpetrello ryanpetrello merged commit 65cd84a into pecan:master Jul 10, 2025
24 checks passed
@kajinamit kajinamit deleted the no-mutable-defaults branch July 14, 2025 07:52
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.

2 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载