Skip to content

Add name property to ModelComponents, DiffusionModels and ComponentCollections#178

Open
henrikjacobsenfys wants to merge 7 commits into
developfrom
add-name-property-for-easylist-migration
Open

Add name property to ModelComponents, DiffusionModels and ComponentCollections#178
henrikjacobsenfys wants to merge 7 commits into
developfrom
add-name-property-for-easylist-migration

Conversation

@henrikjacobsenfys
Copy link
Copy Markdown
Member

This is needed for migrating to using EasyList #117 #70

@henrikjacobsenfys henrikjacobsenfys added [scope] enhancement Adds/improves features (major.MINOR.patch) [priority] medium Normal/default priority labels May 10, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (develop@687e079). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff             @@
##             develop     #178   +/-   ##
==========================================
  Coverage           ?   98.18%           
==========================================
  Files              ?       47           
  Lines              ?     3140           
  Branches           ?      573           
==========================================
  Hits               ?     3083           
  Misses             ?       35           
  Partials           ?       22           
Flag Coverage Δ
integration 45.98% <45.23%> (?)
unittests 98.18% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Member

@rozyczko rozyczko left a comment

Choose a reason for hiding this comment

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

  • ComponentCollection.__init__ still has display_name defaulting to a string, which is inconsistent with the rest, where it defaults to None (or name)

  • DiffusionModelBase has a separate __repr__ that doesn't include name

Comment on lines 47 to 48
self._unit = unit

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

EasyDynamicsModelBase.__init__ already calls self._unit = _validate_unit(unit), so the second assignment overwrites it without validation. It's better to remove this assignment entirely.

@henrikjacobsenfys
Copy link
Copy Markdown
Member Author

@rozyczko would you give a quick thumbs up? :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[priority] medium Normal/default priority [scope] enhancement Adds/improves features (major.MINOR.patch)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants