Serialization: code cleanup

[fingolfin]

Some trivial "code beautification" (obviously this is subjective).

Mostly this is a result of me trying to understand the code. More important than the changes in here (which in the end are not important at all) are some questions about the code logic that I'll ask below -- so really I am abusing this PR to be able to point to parts of the code and discuss them ;-).

[antonydellavecchia]

looks fine by me,..., but I guess I should wait for your questions?

[fingolfin]

Looking at this code, I wonder: why is there save_type_params? Could most of those be turned into save_object methods?

Just these two would be dropped without direct replacement:

function save_type_params(s::SerializerState, obj::Any, key::Symbol)
  set_key(s, key)
  save_type_params(s, obj)
end

function save_type_params(s::SerializerState, obj::T) where T
  save_type_params(s, type_params(obj))
end

and then some save_type_params(s, obj) will have to be replaced by save_object(s, type_params(obj))

The story might be a bit different for load_type_params -- I have a hard time telling because I still am trying to wrap my brain around how exactly those types getting computer :-/

[antonydellavecchia]

the idea is save_object only saves "data" as in no type information but also no save_object should

call a save_type_params anywhere in one of it's child save methods

or do you mean in this file?

[fingolfin]

please have a look @antonydellavecchia

[antonydellavecchia]

these lines are from 2023, polymake objects don't get ids anymore, so this can be totally removed.

[fingolfin]

Why is this comment relevant? The code below it doesn't care whether we look at a container or not

[antonydellavecchia]

it's because there was a lot of recursion and jumping around between different parts of the code here so I left the comment to sort of ground me.

and essential line 287 is only for containers?
maybe it would be cleaner if we had a union type for containers? and I could just check if the type was a subtype on 287

[fingolfin]

This is another case were save_type_params versus save_object has to be explicitly handled, which wouldn't be necessary if save_type_params was just a save_object method -- though here it is save_typed_object that is being called on the params ?!? Huh.

[antonydellavecchia]

so continuing from my response above.
the point is save_type_params may end in a save_typed_object at some point,

for example we need to store the parent ring inside the type and parameter section of the file,
so we will need to get to a save_typed_object on the parent at somepoint

we strictly do not allow this in save_objects since we want all type information to come at the front of the file in one place, this way we can load all type information before we start to load the data section.

the idea is that tere are two branches from the root, one for the type and paramters and one for the data.

save_type_params / load_type_params recurse on the type and parameters branch
save_object / load_object recurse on the data branch