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

Refine MemorySize class #249

Merged
merged 6 commits into from
May 28, 2024
Merged

Refine MemorySize class #249

merged 6 commits into from
May 28, 2024

Conversation

Scienfitz
Copy link
Collaborator

@Scienfitz Scienfitz commented May 24, 2024

A small refinement of the MemorySize class which renames attributes and adds the memory size in bytes (compared to the human readable format this is useful for programmatic evaluation of the mem size)

one alternative way of designing the class would be to make the *_human_readable items properties, but these are not visible when simply printing the entire memory size object

@Scienfitz Scienfitz added the enhancement Expand / change existing functionality label May 24, 2024
@Scienfitz Scienfitz self-assigned this May 24, 2024
Copy link
Collaborator

@AdrianSosic AdrianSosic left a comment

Choose a reason for hiding this comment

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

Hi @Scienfitz, in fact, I'd very much prefer the version you've mentioned in the PR description, with the human_readable stuff as properties. That way, we'd only have the two bytes attributes + shape attributes, and the property could return a simple tuple with the adjusted size and unit. And we can easily adjust the __str__ method if you think that's necessary.

@Scienfitz Scienfitz force-pushed the feature/refine_memorysize branch 3 times, most recently from 5023ad7 to e24a093 Compare May 28, 2024 10:22
@Scienfitz Scienfitz merged commit 959780b into main May 28, 2024
9 of 10 checks passed
@Scienfitz Scienfitz deleted the feature/refine_memorysize branch May 28, 2024 14:46
@Scienfitz Scienfitz restored the feature/refine_memorysize branch May 28, 2024 15:26
@Scienfitz Scienfitz deleted the feature/refine_memorysize branch May 28, 2024 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Expand / change existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants