🔧 Typing of ReqIFSpecHierarchy.children

[ubmarco]

No description provided.

[stanislaw]

@ubmarco thanks for contributing. It looks like your test is failing due to an unrelated change.

I will both the unrelated change and some warnings that I am also seeing here.

After I ping you, could you fix your change and rebase-squash your commit to just one?

[stanislaw]

Please disregard my original comment above and check how I updated it ☝️

[stanislaw]

@ubmarco I have fixed the failing mypy check here: #194. Please rebase/squash your PR to just one commit and we are good to go.

[ubmarco]

Rebased and squashed to 1 commit. I was unhappy with the nullable list, so I made the list mandatory.
Downstream tools do not have to check for None when adding children. Also helps with type checkers.

[stanislaw]

Rebased and squashed to 1 commit. I was unhappy with the nullable list, so I made the list mandatory. Downstream tools do not have to check for None when adding children. Also helps with type checkers.

Thanks! Making the list mandatory could contradict to the approach that I was following so far where the nullability also mirrors to whether the corresponding XML elements will be generated or not. I believe you may now hit some failing test cases due to this because all spec hierarchies will now be generated with empty children which I think is not what we want for a general case.

[stanislaw]

So I feel like your initial change with creating the [] in the add method would work very well.

[ubmarco]

Thanks for the insights. Reverted it.

Usage as a library with type hinting is still not optimal:

spec_hierarchy = ReqIFSpecHierarchy()
    identifier=create_uuid(),
    spec_object=spec_object.identifier,
    level=1,
)
if len(spec_hierarchy.children) > 2:
    pass

leads to

Argument of type "List[Unknown] | None" cannot be assigned to parameter "obj" of type "Sized" in function "len"
  Type "List[Unknown] | None" is not assignable to type "Sized"
    "None" is incompatible with protocol "Sized"
      "__len__" is not present

I would actually make it mandatory as instance attribute, then have an option how to unparse/serialize empty lists (leave out or create an empty list).

[stanislaw]

Thanks for the insights. Reverted it.

Usage as a library with type hinting is still not optimal:

spec_hierarchy = ReqIFSpecHierarchy()
    identifier=create_uuid(),
    spec_object=spec_object.identifier,
    level=1,
)
if len(spec_hierarchy.children) > 2:
    pass

leads to

Argument of type "List[Unknown] | None" cannot be assigned to parameter "obj" of type "Sized" in function "len"
  Type "List[Unknown] | None" is not assignable to type "Sized"
    "None" is incompatible with protocol "Sized"
      "__len__" is not present

I would actually make it mandatory as instance attribute, then have an option how to unparse/serialize empty lists (leave out or create an empty list).

In your example above, you would need to add an is not None check and the error would go away.

The idea behind this parse/unparse was that literally any ReqIF will be parsed/unparsed with the default settings. One big part of it is that some files have empty tags and some not. If you would push this detail down to the unparser level, you would then have to provide a special option when dealing with any arbitrary file which I think is not what we want. I wish I could do better with explaining this in the doc but that's pretty much the idea: the Python object model mirrors the XML tags:

If there is no tag, it is a None.
If there is a tag but no children, it is [].

[stanislaw]

Having the above said, your PR as it is now makes a very good sense. I just noticed this unneeded from __future__ import annotations: please remove it and we will be good to go. I could do it myself but don't want to interfere with you pushing just in case.

[ubmarco]

This is a circular type reference, i.e. the type is not yet defined when using.
2 options:

• as I did with the import
• use quotes around the type, so it becomes a string (also ok for me)

PEP 563 offers the future import to get rid of the quotes for Python <3.14.

[stanislaw]

Ah! I see now. Yes, it is ok to use quotes in your PR, and this is how I did it until now in reqif and the parent strictdoc. If we wanted to handle this project-wise, we could discuss it as a separate change. But I really don't mind having quotes since the code is all quote-based since long ago.

[ubmarco]

Done

[stanislaw]

I could release a new Pip later today (the process is still semi-manual).

Were you planning to contribute any further PRs right away? Otherwise, I would just release this specific change.

[ubmarco]

Sure, release whenever it suits you. Thanks for the help getting this merged. Typing is quality of life :)