Open dset0x opened 9 years ago
intbitset(trailing_bits=True)
is an infinite structure that can't be converted to list. IMHO this is r_wontfix
issue.
The question here is: why we are converting in todict
? All I requested in my code was a dictionary of my entry.
@dset0x you have answered yourself - because you are requesting a dict
.
@dset0x you have answered yourself - because you are requesting a dict.
A dictionary can hold this object just like any other:
In [75942]: {'infinity': intbitset(trailing_bits=True)}
Out[75942]: {'infinity': intbitset([0], trailing_bits=True)}
see https://github.com/inveniosoftware/invenio-ext/blob/master/invenio_ext/sqlalchemy/utils.py#L133-L135
Iterables should too.
intbitset
has it's own handling because we were not able to create a JSON out of it. Maybe it's time to use better JSON encoder/decorer that can handle it. See also https://github.com/inveniosoftware/invenio-ext/blob/master/invenio_ext/sqlalchemy/utils.py#L115-L116
There is still a question: Why do you need to store an intbitset with trailings_bit=True
in database?
This structure does not have any Python equivalent except of an infinite generator.
There is still a question: Why do you need to store an intbitset with trailings_bit=True in database?
I wish to express infinity, therefore I store a dump of an expression of that (in the same format that I store non-infinity), instead of another magic value. This keeps my TypeDecorator above clean. I only afterwards interpret what infinity means for my logic. I do not necessarily iterate over it.
intbitset has it's own handling because we were not able to create a JSON out of it.
I don't understand what JSON has to do with a method that is called when __builtin__.dict
is called. I do not wish to serialize my data. Perhaps this was added assuming that the only reason to call __builtin__.dict
was to serialize?
(a) use the model directly (b) serialize only fields you really need (c) fix the serialization and check all dependent code
(a) use the model directly
I wanted a dictionary of the entire object, like SQLAlchemy normally hands over.
(b) serialize only fields you really need
Again, I am not serializing anything (But yes, I want this field in my dictionary).
(c) fix the serialization and check all dependent code
Since there is no reason to do these conversions, then this is the solution that will prevent future pain. I suspect however, that removing this code will break edge cases in all kinds of hard-to-find code, especially since __iter__
was overriden as well.
The use-cases are not obvious to me even in inveniosoftware/invenio@1ee2c0ce93cd4976aff9062baf973c1e094c4962, where this code seems to originate.
Perhaps someone can come up with a better plan to revert this.
@jirikuncar indeed re-reading all this issue and the referred original code, seems like that the .todict()
method was indeed trying to do too much. Looks like was more a .tojsondict()
i.e. returning something that is safely serializable to JSON. However as @dset0x points out, the .todict()
method is called indirectly also in contexts where JSON serialization is not needed, and where a real instance of an intbitset
is perfectly OK.
So in your opinion can .todict()
be amended to not be too smart and simply return a dictionary with values not converted/serialized?
We can amend it as anyway the intbitset
will not be necessary for core packages anymore.
Given an infinite
intbitset
,and this SQLAlchemy type,
calling
dict()
on an instance that contains it, raises this exception:intbitset
behaves reasonably here, but this conversion was not asked for. Why are these conversions forced intodict
?