[rozyczko]

Map has gone from a typed object graph to a plain weak-reference registry indexed by unique_name.

Current structure of Map:

class Map:
    _store: weakref.WeakValueDictionary   # unique_name -> object

    vertices() -> list[str]          # all registered names
    objects()  -> list[object]       # all live objects (new)
    get_item_by_key(name) -> object  # raises ValueError if missing
    add_vertex(obj) -> None          # raises ValueError on duplicate name
    prune(name) -> None              # pop if present, silent otherwise
    _clear() -> None                 # empty the store
    __repr__

• There is no second dict to maintain with _store. This removes the stale entry cleanup in add_vertex and in the finalizer callbacks. WeakValueDictionary deletes the stale entries, so iterating the list over it is safe during changes.
• get_item_by_key does a single lookup with try/except KeyError instead of checking and then indexing.
• prune used to delete only when the name was in __type_dict. It now always does _store.pop(key).
• objects() returns objects as a list of strong references, so they stay alive until the list is released.
• Renaming keeps the old name. The unique_name setter calls add_vertex again without pruning the old name. A renamed object is registered with both names and can appear twice in objects(). Its old name still shows in vertices(). Explained in docstrings.
• The duplicate-name error now says "map" instead of "graph".

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.78%. Comparing base (adaed61) to head (45a26fe).
⚠️ Report is 5 commits behind head on release-3.0.

Additional details and impacted files

@@               Coverage Diff               @@
##           release-3.0     #338      +/-   ##
===============================================
- Coverage        93.99%   93.78%   -0.22%     
===============================================
  Files               48       48              
  Lines             3946     3764     -182     
===============================================
- Hits              3709     3530     -179     
+ Misses             237      234       -3     

Flag	Coverage Δ
unittests	93.78% <100.00%> (-0.22%)	⬇️

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

Files with missing lines	Coverage Δ
src/easyscience/base_classes/model_base.py	100.00% <100.00%> (ø)
src/easyscience/base_classes/new_base.py	100.00% <100.00%> (ø)
src/easyscience/global_object/__init__.py	100.00% <100.00%> (ø)
src/easyscience/global_object/global_object.py	100.00% <100.00%> (ø)
src/easyscience/global_object/object_registry.py	100.00% <100.00%> (ø)
src/easyscience/variable/parameter.py	95.22% <100.00%> (ø)

... and 1 file with indirect coverage changes

[rozyczko]

I have second thoughts about this. vertex is an old term used in the tree representation. We shouldn't really be using it.

Maybe add/modify with some more obvious syntax, like

    def __getitem__(self, name: str) -> object:
        return self._store[name]  

    def __contains__(self, name: object) -> bool:
        return name in self._store

    def __iter__(self):
        return iter(list(self._store)) 

    def __len__(self) -> int:
        return len(self._store)

    def names(self) -> list[str]: ...         # was vertices()
    def objects(self) -> list[object]: ...    # unchanged
    def add(self, obj) -> None: ...           # was add_vertex()
    def discard(self, name: str) -> None: ... # was prune()
    def clear(self) -> None: ...              # was _clear()

THis adds support for in, len and [] and iterations.

[damskii9992]

Yes, I support renaming the methods to a more dict-like or list-like representation. I would also rename the object itself, maybe to IdTable or something, since it is not really a map any longer. GlobalRegistry? IdRegistry? CPR? ;)

[rozyczko]

Yes, I support renaming the methods to a more dict-like or list-like representation. I would also rename the object itself, maybe to IdTable or something, since it is not really a map any longer. GlobalRegistry? IdRegistry? CPR? ;)

I would propose ObjectRegistry for class name. It fits the behaviour and the attribute makes the meaning obvious.
global_object.registry['par_1']

[damskii9992]

Small comments. Pending renaming of class and methods for full approval.

[damskii9992]

I would probably keep the comment. It's useful for newcomers who don't know what a WeakValueDictionary is.

[rozyczko]

I kept the second comment only as they overlapped.

[rozyczko]

Small comments. Pending renaming of class and methods for full approval.

Addressed.

[damskii9992]

You merged this PR before approval . . .
Didn't we also talk about freeing up unique_names upon renaming in this PR? You didn't add that . . .

[rozyczko]

You merged this PR before approval . . . Didn't we also talk about freeing up unique_names upon renaming in this PR? You didn't add that . . .

You said:

Pending renaming of class and methods for full approval.

both renamings were done. I assumed approval on this condition.

freeing up unique_names is another PR for clarity.