Skip to content

Replaced CollectionBase with EasyList #304 - #305

Open
rozyczko wants to merge 3 commits into
sampler-engine-structure-280from
easylist-on-multifitter
Open

Replaced CollectionBase with EasyList #304#305
rozyczko wants to merge 3 commits into
sampler-engine-structure-280from
easylist-on-multifitter

Conversation

@rozyczko

@rozyczko rozyczko commented Sep 4, 2026

Copy link
Copy Markdown
Member

This pull request replaces the deprecated CollectionBase container with the new EasyList for aggregating fit objects in MultiFitter.

@rozyczko rozyczko added [scope] maintenance Code/tooling cleanup, no feature or bugfix (major.minor.PATCH) [priority] high Should be prioritized soon [area] base classes Changes to or creation of new base classes labels Sep 4, 2026
@codecov

codecov Bot commented Sep 4, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.66%. Comparing base (693e11c) to head (f646049).
⚠️ Report is 5 commits behind head on sampler-engine-structure-280.

Additional details and impacted files

Impacted file tree graph

@@                       Coverage Diff                        @@
##           sampler-engine-structure-280     #305      +/-   ##
================================================================
- Coverage                         83.82%   83.66%   -0.16%     
================================================================
  Files                                68       68              
  Lines                              5348     5272      -76     
================================================================
- Hits                               4483     4411      -72     
+ Misses                              865      861       -4
Flag Coverage Δ
unittests 83.66% <100.00%> (-0.16%) ⬇️

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

Files with missing lines Coverage Δ
src/easyscience/fitting/fitter.py 91.33% <ø> (-0.20%) ⬇️
src/easyscience/fitting/multi_fitter.py 98.71% <100.00%> (+0.12%) ⬆️

... and 13 files with indirect coverage changes

damskii9992
damskii9992 approved these changes Sep 9, 2026
View reviewed changes

@damskii9992 damskii9992 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Small comments only, otherwise it looks good to me.

Comment thread src/easyscience/fitting/multi_fitter.py Outdated
Comment on lines 28 to 29
fit_objects: list | None = None,
fit_functions: list[Callable] | None = None,

@damskii9992 damskii9992 Sep 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be typehinted as sequence, since we support any sequence?

@rozyczko rozyczko Sep 11, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes. And more. We have to properly initialize fit_objects and fit_functions, otherwise the code will crash on the subsequent indexing... oops. Fixed.

@damskii9992 damskii9992 Sep 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You still didn't change the typehint lol.

@rozyczko rozyczko Sep 15, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What do you mean I didn't... I just did! Now.

Comment thread src/easyscience/fitting/fitter.py Outdated
Comment thread tests/unit/fitting/test_multi_fitter.py Outdated
Comment thread tests/unit/fitting/test_multi_fitter.py Outdated
Comment thread tests/unit/fitting/test_multi_fitter.py Outdated
@rozyczko rozyczko mentioned this pull request Sep 15, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@damskii9992 damskii9992 damskii9992 approved these changes

Assignees

No one assigned

Labels

[area] base classes Changes to or creation of new base classes [priority] high Should be prioritized soon [scope] maintenance Code/tooling cleanup, no feature or bugfix (major.minor.PATCH)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@rozyczko @damskii9992