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

Remove private _ModuleBuilderDomain #589

Merged

Conversation

lekcyjna123
Copy link
Contributor

Hi,
based on discussion from #577, it looks like _ModuleBuilderDomain usage in transactron is not needed.

  • typing information give us nothing, because _ModuleBuilderDomain in amaranth is simply a wrapper, which puts data to the correct place
  • in __setattr__ - as I understand - we check for the type to make sure that we are doing modification in place. This can be abstracted away with our internal class.

@tilk
Copy link
Member

tilk commented Feb 16, 2024

I would much rather prefer a protocol over a wrapper. Edit: but it looks like using a protocol would create problems for the check in __setattr__. A wrapper is fine then.

transactron/core.py Outdated Show resolved Hide resolved
transactron/core.py Outdated Show resolved Hide resolved
Co-authored-by: Marek Materzok <[email protected]>
transactron/core.py Outdated Show resolved Hide resolved
transactron/core.py Outdated Show resolved Hide resolved
Lekcyjna added 2 commits February 20, 2024 18:48
…kcyjna123/coreblocks into lekcyjna/remove-module-builder-domain
@tilk tilk merged commit 5a7390c into kuznia-rdzeni:master Feb 27, 2024
8 checks passed
github-actions bot pushed a commit that referenced this pull request Feb 27, 2024
tilk pushed a commit to kuznia-rdzeni/transactron that referenced this pull request Nov 25, 2024
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.

3 participants