ion-python icon indicating copy to clipboard operation
ion-python copied to clipboard

Symbols lack SIds when C-extension is enabled

Open rmarrowstone opened this issue 2 years ago • 2 comments

What I expect:

>>> from amazon.ion.simpleion import loads
>>> loads('$1')
IonPySymbol(text='$ion', sid=1, location=ImportLocation(name='$ion', position=1))
>>>
>>>
>>>

What happens:

>>> from amazon.ion.simpleion import loads
>>> loads('$1')
IonPySymbol(text='$ion', sid=None, location=None)
>>>
>>>
>>>

Confirming this is c-extension specific (continued from same repl session above):

>>> from amazon.ion import simpleion
>>> simpleion.c_ext
True
>>> simpleion.c_ext = False
>>> loads('$1')
IonPySymbol(text='$ion', sid=1, location=ImportLocation(name='$ion', position=1))
>>>

rmarrowstone avatar Dec 13 '23 18:12 rmarrowstone

All three variants are "correct", in that when the text is known, nothing else matters. Is this a concern about consistency across the reader options?

tgregg avatar Dec 13 '23 19:12 tgregg

~~When the text is not known, the information about SId and import location is also missing.~~

~~I see. That currently raises an error as well, though we could be returning a SymbolToken with None text and Sid and import location.~~

Update: there is a correctness issue regarding imported Symbols with undefined text. As it is now, imported symbols with undefined text are considered equivalent to each other (regardless of SId) and local symbols with undefined text.

rmarrowstone avatar Dec 22 '23 18:12 rmarrowstone