Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Symbols lack SIds when C-extension is enabled #316

Open
rmarrowstone opened this issue Dec 13, 2023 · 2 comments
Open

Symbols lack SIds when C-extension is enabled #316

rmarrowstone opened this issue Dec 13, 2023 · 2 comments
Labels

Comments

@rmarrowstone
Copy link
Contributor

rmarrowstone commented Dec 13, 2023

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))
>>>
@tgregg
Copy link
Contributor

tgregg commented Dec 13, 2023

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?

@rmarrowstone
Copy link
Contributor Author

rmarrowstone commented Dec 22, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants