Repository navigation
Implement hasattr(), str() and repr(). - #8442
Conversation
This patch: * Implements the `hasattr()` builtin. * Reimplements the `str()` builtin to behave more like Python in that `__str__` and `repr` are used internally. * Implements `repr` through the use of `__repr__`. * Fixes up `hash` to check the result of a `__hash__` method before returning and adds some more hash implementations. * Refactors a common "create a dummy type" method into the standard Numba `numba.tests.support.TestCase`. * Updates the `LiteralPropagation` compilation pass to recognised inferred constants from `hasattr` calls.
| # There's no __str__ or __repr__ defined for this object, go via | ||
| # the interpreter. | ||
| with objmode(pyrepr='unicode_type'): | ||
| pyrepr = _get_py_repr_and_warn(obj) | ||
| return pyrepr |
There was a problem hiding this comment.
TODO: Need to decide about this! Another option is to raise.
There was a problem hiding this comment.
one more option is to print something like <object XYZType> so it doesn't need objmode
There was a problem hiding this comment.
From Numba public meeting discussion on 2023-01-10, the use of objmode is going to be replaced with a simple compiled in string cf. #8442 (comment). This is because users are potentially going to str() or repr() objects to essentially stringify them for use elsewhere and reacquiring the GIL in such a case would be an unnecessary performance penalty.
There was a problem hiding this comment.
This is implemented and tested in b819f01.
guilhermeleobas
left a comment
There was a problem hiding this comment.
LGMT. I only have one concern regarding hasattr. Although the impl. for it seems correct. It might produce false negatives in due to how Numba overloads some functions:
@njit
def test():
s = set([1, 2, 3])
return hasattr(s, '__len__'), len(s)
print(test())Should Numba emit a warning for users to use this feature with caution? Or even an acceptlist of functions we know are safe to use (str/repr/hash). Other functions should be safe once one migrates @overload(func) to @overload_method(type, func)
|
Thanks for the review @guilhermeleobas. Two remarks:
|
|
I searched for
|
Thanks @guilhermeleobas, from memory, I think that Numba has implementations for a reduced set of these, namely:
I'm not sure if it's possible to handle |
|
In the interests of making this feature available for Numba 0.57, what needs to be done to get this patch to a state where it's sufficiently useful to be able to merge it? Is it potentially "just" resolving these questions: #8442 (comment) ? I don't think that it's going to be possible to handle quite a few of those dunder methods above trivially in this PR. For example |
sklam
left a comment
There was a problem hiding this comment.
Besides the minor suggestion to the error message in the untyped_passes.py, the main question is why allow __hash__ to return None instead of raising.
Once false negatives problems and the above suggestions are addressed, i think this patch is safe to be included.
This fixes the issue noted in review where the `getattr(obj, __hash__)` call could return a function that when executed returned None, which is an invalid type for a hash.
As title. Also adds test.
|
Thanks for the review @sklam, comments should now be addressed. |
|
1c5970d failed in Azure CI on the This doesn't reproduce locally. Rerunning the CI job to see if it's transient. |
It's not transient. |
|
/AzurePipelines run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
9ef54ce fixes the transient issue. |
|
|
Resolved conflicts in: numba/tests/support.py
As title. This replaces the objmode fall back with a generic string referring to an object of the type of the argument.
* `f-string`s use `str()` which now doesn't raise at typing time. * One of the usecases tests relied on objmode so force it as `str()` will now always manage to compile in `nopython` mode.
This patch:
hasattr()builtin.str()builtin to behave more like Python in that__str__andreprare used internally.reprthrough the use of__repr__.hashto check the result of a__hash__method before returning and adds some more hash implementations.numba.tests.support.TestCase.LiteralPropagationcompilation pass to recognised inferred constants fromhasattrcalls.