Skip to content
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

periph: add a ConstantMap container for configuration constants. #25

Merged
merged 1 commit into from Aug 17, 2020

Conversation

jfng
Copy link

@jfng jfng commented Aug 17, 2020

Resolves #24.

Copy link
Member

@whitequark whitequark left a comment

LGTM overall, feel free to merge with or without my suggestion for iteration order.

class ConstantMap(Mapping):
"""Named constant map.

A read-only container for named constants. Keys are iterated in ascending order.
Copy link
Member

@whitequark whitequark Aug 17, 2020

Choose a reason for hiding this comment

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

I don't have a strong opinion on this, but I feel like insertion order would be more useful--this way the order of constants in the BSP matches the order of constants in peripheral code. This would matter for few people (most won't be looking at raw BSP sources), but it still seems nice; on the other hand, any deterministic iteration order is fine reproducibility-wise.

@jfng jfng merged commit b405880 into amaranth-lang:master Aug 17, 2020
4 checks passed
@jfng
Copy link
Author

@jfng jfng commented Aug 17, 2020

Thanks! Turns out **kwargs preserves insertion order in a function starting from Python 3.6, so making the change was straightforward.

@jfng jfng deleted the constant-map branch Aug 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants