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

fix: fix IPython inspection #2658

Merged
merged 1 commit into from
Aug 18, 2023
Merged

Conversation

agoose77
Copy link
Collaborator

@agoose77 agoose77 commented Aug 18, 2023

In IPython, the wildcard magic:

In [1]: import awkward as ak

In [2]: x = ak.Array({'x': [1,2,3]})

In [3]: x.*?
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
File ~/.mambaforge/envs/awkward/lib/python3.11/site-packages/IPython/core/oinspect.py:1124, in Inspector.psearch(self, pattern, ns_table, ns_search, ignore_case, show_all, list_types)
   1122         continue
   1123     namespaces_seen.add(id(ns))
-> 1124     tmp_res = list_namespace(ns, type_pattern, filter,
   1125                             ignore_case=ignore_case, show_all=show_all)
   1126     search_result.update(tmp_res)
   1128 page.page('\n'.join(sorted(search_result)))

File ~/.mambaforge/envs/awkward/lib/python3.11/site-packages/IPython/utils/wildcard.py:106, in list_namespace(namespace, type_pattern, filter, ignore_case, show_all)
    104 results = {}
    105 for name, obj in filtered.items():
--> 106     ns = list_namespace(dict_dir(obj), type_pattern,
    107                         ".".join(pattern_list[1:]),
    108                         ignore_case=ignore_case, show_all=show_all)
    109     for inner_name, inner_obj in ns.items():
    110         results["%s.%s"%(name,inner_name)] = inner_obj

File ~/.mambaforge/envs/awkward/lib/python3.11/site-packages/IPython/utils/wildcard.py:70, in dict_dir(obj)
     62 for key in dir2(obj):
     63    # This seemingly unnecessary try/except is actually needed
     64    # because there is code out there with metaclasses that
   (...)
     67    # object's dictionary.  Properties can actually do the same
     68    # thing.  In particular, Traits use this pattern
     69    try:
---> 70        ns[key] = getattr(obj, key)
     71    except AttributeError:
     72        pass

File ~/Git/awkward/src/awkward/highlevel.py:1326, in Array.__cuda_array_interface__(self)
   1321 @non_inspectable_property
   1322 def __cuda_array_interface__(self):
   1323     with ak._errors.OperationErrorContext(
   1324         f"{type(self).__name__}.__cuda_array_interface__", (self,), {}
   1325     ):
-> 1326         array = ak.operations.to_cupy(self)
   1327         return array.__cuda_array_interface__

File ~/Git/awkward/src/awkward/_dispatch.py:60, in named_high_level_function.<locals>.dispatch(*args, **kwargs)
     58 # Failed to find a custom overload, so resume the original function
     59 try:
---> 60     next(gen_or_result)
     61 except StopIteration as err:
     62     return err.value

File ~/Git/awkward/src/awkward/operations/ak_to_cupy.py:30, in to_cupy(array)
     27 yield (array,)
     29 # Implementation
---> 30 return _impl(array)

File ~/Git/awkward/src/awkward/operations/ak_to_cupy.py:39, in _impl(array)
     36 backend = CupyBackend.instance()
     37 cupy_layout = layout.to_backend(backend)
---> 39 return cupy_layout.to_backend_array(allow_missing=False)

File ~/Git/awkward/src/awkward/contents/content.py:1077, in Content.to_backend_array(self, allow_missing, backend)
   1075 else:
   1076     backend = regularize_backend(backend)
-> 1077 return self._to_backend_array(allow_missing, backend)

File ~/Git/awkward/src/awkward/contents/recordarray.py:1120, in RecordArray._to_backend_array(self, allow_missing, backend)
   1117     raise ValueError(f"cannot convert {self} into np.ndarray")
   1119 if not backend.nplike.supports_structured_dtypes:
-> 1120     raise TypeError(
   1121         f"backend {backend.name!r} does not support structured "
   1122         f"dtypes required for converting {type(self).__name__} "
   1123         f"into a backend array"
   1124     )
   1125 out = backend.nplike.empty(
   1126     self.length,
   1127     dtype=[(str(n), x.dtype) for n, x in zip(self.fields, contents)],
   1128 )
   1129 mask = None

TypeError: backend 'cuda' does not support structured dtypes required for converting RecordArray into a backend array

This error occurred while calling

    Array.__cuda_array_interface__(
        <Array [{x: 1}, {x: 2}, {x: 3}] type='3 * {x: int64}'>
    )

Throws an error due to the fact that it evaluates the properties to ensure that they exist. This is problematic for properties like cpp_type that throw exceptions if libraries are missing (and are slow). We can detect whether we're in the IPython inspection context, and return None for these properties.

@codecov
Copy link

codecov bot commented Aug 18, 2023

Codecov Report

Merging #2658 (9a261bc) into main (5530433) will decrease coverage by 0.01%.
The diff coverage is 75.00%.

Additional details and impacted files
Files Changed Coverage Δ
src/awkward/highlevel.py 76.11% <75.00%> (-0.18%) ⬇️

@agoose77 agoose77 temporarily deployed to docs-preview August 18, 2023 11:25 — with GitHub Actions Inactive
Copy link
Member

@jpivarski jpivarski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if the library in question is installed, this will make the non-inspectable properties return None if the caller is IPython.

How robust is the test? Is

inspect.currentframe().f_back.f_code is IPython.utils.wildcard.dict_dir.__code__

relying on an implementation detail that could change in the future?

It's highly unlikely/probably impossible for this test to result in false positives, in which someone wants the value of the property and they get None because the predicate returns True when it's not supposed to. If this were not the case, I would add a try-except to check for the library (numba, cppyy).

I suppose the worst thing that could happen is for IPython to change their implementation, this test gives false negatives, and then we're back to having wildcards fail until someone notices and puts in an IPython version-dependent fix. Considering the present state, that's an improvement.

As it is, this is a simple fix and was done 4 hours ago, so I'll merge it now.

@jpivarski jpivarski merged commit 01cbec2 into main Aug 18, 2023
34 of 35 checks passed
@jpivarski jpivarski deleted the agoose77/feat-non-inspectable-property branch August 18, 2023 16:12
@agoose77
Copy link
Collaborator Author

Yes, this is fragile in that it couples to the implementation that IPython uses to inspect objects for the .*? magic. However, as you note, the IPython code is fairly stable, and this PR's failure mode is just the existing behavior :)

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

Successfully merging this pull request may close these issues.

None yet

2 participants