fontParts icon indicating copy to clipboard operation
fontParts copied to clipboard

Discrepancy in native object identifier return

Open knutnergaard opened this issue 1 year ago • 3 comments

base.IdentifierMixin.identifier is a read-only attribute which returns str or None. The docs for the _get_identifier method states that an identifier should be assigned to any object that does not have one. IOW, it should always return str. This does not, however, seem to comply with the base getter:

def _get_base_identifier(self) -> Optional[str]:
        value = self._get_identifier()
        if value is not None:
            value = normalizers.normalizeIdentifier(value)
        return value

Given that the attribute is a mandatory override, should the documentation clarify that the _get_identifier return may be either str or None or am I missing something with regard to the logic here?

knutnergaard avatar Oct 02 '24 12:10 knutnergaard

iirc, None is there for reasons (@typemytype or @typesupply can shed light), so it should clarify that it can return either. Thank you!

benkiel avatar Oct 02 '24 14:10 benkiel

Right, if the identifier hasn't been defined None is returned. If it has been defined, it is passed through the normalizer. We do this because if this property created an identifier if one isn't defined, all objects in all UFOs would have identifiers after their next save. That would do bad things to version control and file sizes.

typesupply avatar Oct 03 '24 13:10 typesupply

To sum, docs should be clear that _get_identifer may return str or None

benkiel avatar Oct 09 '24 01:10 benkiel

@knutnergaard ok to close this?

benkiel avatar Nov 01 '24 16:11 benkiel