Skip to content

Finish lib.memory documentation #1228

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 9 commits into from
Mar 22, 2024
Merged

Conversation

whitequark
Copy link
Member

This PR carries two "minor" semantic changes as well:

  • Removing Memory.Init.depth.
  • Renaming Memory.{r,w}_ports to .{read,write}_ports.

See the commit messages for details.

@whitequark whitequark added this to the 0.5 milestone Mar 22, 2024
@whitequark whitequark marked this pull request as draft March 22, 2024 21:22
Copy link

codecov bot commented Mar 22, 2024

Codecov Report

Attention: Patch coverage is 98.83721% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 89.78%. Comparing base (8c65a79) to head (2f1823e).
Report is 5 commits behind head on main.

Files Patch % Lines
amaranth/lib/memory.py 98.83% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1228      +/-   ##
==========================================
+ Coverage   89.65%   89.78%   +0.12%     
==========================================
  Files          43       43              
  Lines        9718     9923     +205     
  Branches     2331     2395      +64     
==========================================
+ Hits         8713     8909     +196     
- Misses        816      822       +6     
- Partials      189      192       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@whitequark whitequark force-pushed the memory-docs branch 2 times, most recently from 6bc588a to 25ef347 Compare March 22, 2024 22:21
@whitequark whitequark marked this pull request as ready for review March 22, 2024 22:22
@whitequark whitequark enabled auto-merge March 22, 2024 22:23
@whitequark whitequark added this pull request to the merge queue Mar 22, 2024
@whitequark whitequark removed this pull request from the merge queue due to a manual request Mar 22, 2024
This attribute is fully redundant with `.__len__()`, and is out of place
on a `list`-like container like `Memory.Init`.

The `.shape` attribute, however, provides a unique function.
Display `shape` and `depth` also. `depth` is redundant although useful
for ease of reading (there are always `depth` elements shown), but
`shape` was just lost.
The abbreviated form was initially added to match `lib.fifo`, but it
looks very out of place on `lib.memory`, and we may be moving away from
such heavy use of abbreviations anyway.

While technically a breaking change, these attributes have very narrow
usefulness and so this change qualifies as "minor".
@whitequark whitequark added this pull request to the merge queue Mar 22, 2024
Merged via the queue into amaranth-lang:main with commit 6ce8284 Mar 22, 2024
@whitequark whitequark deleted the memory-docs branch March 22, 2024 23:11
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.

1 participant