add getters to m.submodules #156

nakengelhardt · 2019-07-19T11:46:49Z

This allows retrieving a named submodule after assigning it:

>>> m1 = Module()
>>> m1.submodules.m2 = Module()
>>> m1.submodules.m2
<nmigen.hdl.dsl.Module object at 0x7f2acc2cf240>

It also disallows setting the same submodule name twice:

>>> m1 = Module()
>>> m1.submodules.m2 = Module()
>>> m1.submodules.m2 = Module()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/nengel/nmigen/nmigen/hdl/dsl.py", line 85, in __setattr__
    self._builder._add_submodule(submodule, name)
  File "/home/nengel/nmigen/nmigen/hdl/dsl.py", line 432, in _add_submodule
    raise ValueError("Submodule named '{}' already exists".format(name))
ValueError: Submodule named 'm2' already exists

Item notation is also supported:

>>> m1 = Module()
>>> m1.submodules['m2'] = Module()
>>> m1.submodules['m2']
<nmigen.hdl.dsl.Module object at 0x7f52834464a8>
>>> m1.submodules.m2
<nmigen.hdl.dsl.Module object at 0x7f52834464a8>

whitequark · 2019-07-19T11:49:40Z

nmigen/hdl/dsl.py

@@ -124,7 +130,8 @@ def __init__(self):
        self._ctrl_stack   = []

        self._driving      = SignalDict()
-        self._submodules   = []
+        self._submodules   = {}


Nit: should this be something like self._named_submodules?

Sure, let me change that

codecov · 2019-07-19T11:57:59Z

Codecov Report

Merging #156 into master will increase coverage by 0.05%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #156      +/-   ##
==========================================
+ Coverage   80.67%   80.73%   +0.05%     
==========================================
  Files          32       32              
  Lines        5217     5232      +15     
  Branches     1129     1133       +4     
==========================================
+ Hits         4209     4224      +15     
  Misses        873      873              
  Partials      135      135

Impacted Files	Coverage Δ
nmigen/hdl/dsl.py	`99.68% <100%> (+0.01%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 81e5983...ccd8b20. Read the comment docs.

nakengelhardt · 2019-07-19T12:03:25Z

nmigen/hdl/dsl.py

+            self._anon_submodules.append(submodule)
+        else:
+            if name in self._named_submodules:
+                raise ValueError("Submodule named '{}' already exists".format(name))


Actually, I wasn't sure if this should be a ValueError or an AttributeError or maybe a SyntaxError?

add getters to m.submodules

050d3d1

whitequark reviewed Jul 19, 2019

View reviewed changes

change _submodules to _named_submodules

c945c2d

nakengelhardt commented Jul 19, 2019

View reviewed changes

NameError for duplicate submodule assignment

ccd8b20

whitequark merged commit 698b005 into m-labs:master Jul 19, 2019

nakengelhardt deleted the add-submodules-get branch July 20, 2019 04:16

nmigen-issue-migration mentioned this pull request Jan 27, 2020

add getters to m.submodules amaranth-lang/amaranth#157

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add getters to m.submodules #156

add getters to m.submodules #156

nakengelhardt commented Jul 19, 2019

whitequark Jul 19, 2019

nakengelhardt Jul 19, 2019

codecov bot commented Jul 19, 2019 •

edited

nakengelhardt Jul 19, 2019

whitequark Jul 19, 2019

add getters to m.submodules #156

add getters to m.submodules #156

Conversation

nakengelhardt commented Jul 19, 2019

whitequark Jul 19, 2019

Choose a reason for hiding this comment

nakengelhardt Jul 19, 2019

Choose a reason for hiding this comment

codecov bot commented Jul 19, 2019 • edited

Codecov Report

nakengelhardt Jul 19, 2019

Choose a reason for hiding this comment

whitequark Jul 19, 2019

Choose a reason for hiding this comment

codecov bot commented Jul 19, 2019 •

edited