308 remove name by rozyczko · Pull Request #313 · easyscience/core

rozyczko · 2026-09-25T11:14:22Z

Refactoring of Descriptor and Parameter Naming

Refactored all usages of name to use display_name for Parameter instantiation
Updated code that accesses parameter/descriptor names to use display_name instead of name

Modernization of Descriptor Class Constructors

Changed constructors of DescriptorAnyType and DescriptorArray to remove the name positional argument, making all arguments after value keyword-only.

codecov · 2026-09-25T11:26:56Z

Codecov Report

❌ Patch coverage is 99.09910% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.11%. Comparing base (22513ad) to head (55de2db).
⚠️ Report is 1 commits behind head on release-3.0.

Files with missing lines	Patch %	Lines
src/easyscience/variable/descriptor_number.py	93.33%	1 Missing ⚠️

Additional details and impacted files

@@               Coverage Diff                @@
##           release-3.0     #313       +/-   ##
================================================
+ Coverage        83.82%   94.11%   +10.29%     
================================================
  Files               69       51       -18     
  Lines             5272     4127     -1145     
================================================
- Hits              4419     3884      -535     
+ Misses             853      243      -610

Flag	Coverage Δ
unittests	`94.11% <99.09%> (+10.29%)`	⬆️

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

Files with missing lines	Coverage Δ
src/easyscience/__init__.py	`100.00% <100.00%> (ø)`
src/easyscience/base_classes/__init__.py	`100.00% <100.00%> (ø)`
src/easyscience/base_classes/easy_list.py	`96.89% <100.00%> (+0.64%)`	⬆️
src/easyscience/base_classes/model_base.py	`100.00% <100.00%> (ø)`
src/easyscience/base_classes/new_base.py	`100.00% <100.00%> (ø)`
src/easyscience/base_classes/serializer_base.py	`98.15% <100.00%> (ø)`
...syscience/fitting/calculators/interface_factory.py	`97.32% <100.00%> (+0.04%)`	⬆️
...c/easyscience/fitting/minimizers/minimizer_base.py	`97.95% <ø> (ø)`
.../easyscience/fitting/minimizers/minimizer_bumps.py	`99.35% <ø> (ø)`
...rc/easyscience/fitting/minimizers/minimizer_dfo.py	`97.74% <ø> (ø)`
... and 17 more

... and 2 files with indirect coverage changes

forwarding serialization inclusions

damskii9992

So many lines changed but not actually that much relevant. Just a lot of name being replaced with display_name. In principle we could also just not have the name or display_name in tests or tutorials, but I'm also okay with this replacement.

I AM confused about the arg_spec change though . . .

damskii9992 · 2026-10-03T08:37:39Z

Why is this removed and why was this ever moved to legacy?
A document explaining how serialization works for dependent parameters should still be good to keep somewhere . . .
Even IF it needs to get updated, now that the name attribute is gone.

Also, it should probably be moved to the documentation :)

We don't want stray MD files. I thought the docs already had this info but apparently they don't. Will udpate the content of /doc

damskii9992 · 2026-10-03T08:41:14Z

        class_links = self.__interface_obj.create(model)
        props = model._get_linkable_attributes()
-        props_names = [prop.name for prop in props]
+        props_names = [prop.display_name for prop in props]


This should probably be the unique_name rather than the display_name. Even though the display_name defaults to the unique_name if it hasn't been explicitly set.

name_conversion keys are short calculator-side names like "scale".
The old name was such a non-unique label, and display_name is its replacement.
unique_name has to be unique across the whole program, so two models couldn't both have a "scale".
The real fix would be to match on the attribute's key in the model, but we're redoing the interface anyway.

damskii9992 · 2026-10-03T08:43:54Z

We should probably nix this class as well. It only existed to satisfy EasyCrystallography, which has since been deprecated.

Sure. I'll make this a separate PR to not conflate this.

damskii9992 · 2026-10-03T08:46:01Z

        self,
-        name: str,
        value: Union[list, np.ndarray],
+        *,


Why the wildcard arg? We're not processing this, so having it just makes the API more confusing.

* isn't a wildcard. It makes all the remaining arguments keyword-only. It's deliberate - with name gone, an old call like Parameter('a', 1.0) now properly fails instead of setting 'a' as the value.

Ahh, my bad. I'm not a fan of it though. Forcing keyword-only arguments is annoyingly restrictive. And doing Parameter("a", 1.0) should fail anyways since we check the type of inputs and value cannot be a str.

Hmm, I think I might be in favour of the forcing keyword only arguments anyways, but I would put it after unit, so that you can still create a Parameter or DescriptorNumber fast and easily as:

a = Parameter(2.0, 'cm')

damskii9992 · 2026-10-03T09:05:34Z

+        sign = signature(self.__class__.__init__)
+        return {
+            param.name
+            for param in sign.parameters.values()
+            if param.kind in (param.POSITIONAL_OR_KEYWORD, param.KEYWORD_ONLY)
+            and param.name != 'self'
+        }


Why is this neccesary?
Is the regular arg_spec from the NewBase not good enough? Does it not get all the fields?
If so, we should fix it in NewBase . . . .

as explained below.

rozyczko · 2026-10-04T05:20:08Z

I AM confused about the arg_spec change though . . .

This is needed because NewBase._arg_spec only collects POSITIONAL_OR_KEYWORD, so keyword-only arguments disappear from to_dict.
Making everything after value keyword-only therefore makes those arguments invisible to it.

NewBase._arg_spec      -> ['value']
DescriptorBase._arg_spec -> ['callback','description','display_name','fixed','max','min',
                             'unique_name','unit','url','value','variance']

Without the override, to_dict() would drop unit, variance, min, max, etc. The override adds KEYWORD_ONLY to the filter, so it is necessary.

I guess placing it in NewBase makes even more sense.
EasyList._arg_spec returns a set() now. I just tested it and it seems EasyList(..., display_name='My list').to_dict() loses the display name in the current impl but obviously retains it when in NewBase.

damskii9992 · 2026-10-05T10:12:53Z

I AM confused about the arg_spec change though . . .

This is needed because NewBase._arg_spec only collects POSITIONAL_OR_KEYWORD, so keyword-only arguments disappear from to_dict. Making everything after value keyword-only therefore makes those arguments invisible to it.
NewBase._arg_spec      -> ['value']
DescriptorBase._arg_spec -> ['callback','description','display_name','fixed','max','min',
                             'unique_name','unit','url','value','variance']
Without the override, to_dict() would drop unit, variance, min, max, etc. The override adds KEYWORD_ONLY to the filter, so it is necessary.

I guess placing it in NewBase makes even more sense. EasyList._arg_spec returns a set() now. I just tested it and it seems EasyList(..., display_name='My list').to_dict() loses the display name in the current impl but obviously retains it when in NewBase.

Actually, we should just simply update the get_arg_spec method in the SerializerBase class to use signature to also get KEYWORD_ONLY arguments. That would probably be the best approach.

rozyczko · 2026-10-05T11:46:13Z

I AM confused about the arg_spec change though . . .

This is needed because NewBase._arg_spec only collects POSITIONAL_OR_KEYWORD, so keyword-only arguments disappear from to_dict. Making everything after value keyword-only therefore makes those arguments invisible to it.
NewBase._arg_spec      -> ['value']
DescriptorBase._arg_spec -> ['callback','description','display_name','fixed','max','min',
                             'unique_name','unit','url','value','variance']
Without the override, to_dict() would drop unit, variance, min, max, etc. The override adds KEYWORD_ONLY to the filter, so it is necessary.
I guess placing it in NewBase makes even more sense. EasyList._arg_spec returns a set() now. I just tested it and it seems EasyList(..., display_name='My list').to_dict() loses the display name in the current impl but obviously retains it when in NewBase.
Actually, we should just simply update the get_arg_spec method in the SerializerBase class to use signature to also get KEYWORD_ONLY arguments. That would probably be the best approach.

Hmmm I guess this is slightly better than the existing impl. Fixing the generic path and deleting the NewBase override is cleaner.

Remove _REDIRECT; serializer relies on _arg_spec only

…-objbase

292 remove objbase

rozyczko added 3 commits September 24, 2026 14:47

initial commit

b63c8d3

more fixes after internal CR

3d9bc5d

ruff now changed?

1008789

Merge remote-tracking branch 'origin/release-3.0' into 308-remove-name

31ff48d

rozyczko mentioned this pull request Sep 25, 2026

Remove name attribute from DescriptorBase and its children #308

Closed

minor text fixes

38a8131

rozyczko marked this pull request as ready for review September 25, 2026 13:25

rozyczko requested a review from damskii9992 September 25, 2026 14:28

rozyczko added 8 commits September 27, 2026 22:13

removed vestiges of ObjBase and such

076b0bb

minor fixes for calculator binding, new coefficient container and

8c3aa51

forwarding serialization inclusions

more changes, PR review, removal of Hugger

49f143b

removed Polynomial

2ffde0c

commented out functional tests (Polynomial only)

d270196

removed legacy exclude from pyproject.toml

1be0e78

update unit tests for Codecov

9c00901

clean up the code a tad

94f8293

damskii9992 reviewed Oct 3, 2026

View reviewed changes

rozyczko mentioned this pull request Oct 4, 2026

Remove Descriptor_any_type or move it to legacy #318

Closed

PR review issues addressed

be8c0c9

rozyczko requested a review from damskii9992 October 4, 2026 08:05

rozyczko added 2 commits October 5, 2026 13:52

simplify the new code, as per #313 (comment)

c38c0ea

extensive CR fixes

0f1fb8b

rozyczko added 4 commits October 5, 2026 17:21

Remove _REDIRECT; serializer relies on _arg_spec only

0b71d25

Merge pull request #319 from easyscience/157-remove-redirect-rebased

e5bbff6

Remove _REDIRECT; serializer relies on _arg_spec only

merged 308-remove-name

f071d05

Merge remote-tracking branch 'origin/308-remove-name' into 292-remove…

970f169

…-objbase

rozyczko mentioned this pull request Oct 8, 2026

292 remove objbase #314

Merged

rozyczko added 2 commits October 8, 2026 16:11

Merge pull request #314 from easyscience/292-remove-objbase

363b9f2

292 remove objbase

moved serializer_base from io/ to base_classes/

55de2db

rozyczko merged commit e620f0a into release-3.0 Oct 8, 2026
26 of 28 checks passed

rozyczko deleted the 308-remove-name branch October 8, 2026 16:38

rozyczko mentioned this pull request Oct 30, 2026

Release 3.0 #352

Merged

Conversation

rozyczko commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Refactoring of Descriptor and Parameter Naming

Modernization of Descriptor Class Constructors

Uh oh!

codecov Bot commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

damskii9992 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rozyczko Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rozyczko commented Oct 4, 2026

Uh oh!

damskii9992 commented Oct 5, 2026

Uh oh!

rozyczko commented Oct 5, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rozyczko commented Sep 25, 2026 •

edited

Loading

codecov Bot commented Sep 25, 2026 •

edited

Loading

rozyczko Oct 4, 2026 •

edited

Loading