Skip to content
This repository was archived by the owner on Feb 2, 2024. It is now read-only.

Conversation

@densmirn
Copy link
Contributor

No description provided.

@densmirn densmirn force-pushed the feature/df_getitem_bool_arr branch from 200c54a to 601411e Compare February 16, 2020 09:42
@AlexanderKalistratov
Copy link
Collaborator

@kozlov-alexey could you please review it?

return gen_df_getitem_bool_series_idx_impl(self, idx)

ty_checker.raise_exc(idx, 'str', 'idx')
if isinstance(idx, types.Array) and isinstance(idx.dtype, types.Boolean):
Copy link
Contributor

Choose a reason for hiding this comment

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

I would propose to move it at the top, just after we checked that self is of DataFrameType, because this check should really intercept wrong types (for which we do not want compilation to happen). Otherwise we might compile (or at least try to compile) some specialization we do not want to compile (e.g. if some of the arg types can be type casted to another one).



def df_index_codelines(self):
def df_index_codelines(self, with_length=False):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better for this function to do only index lines generation, so that we just make df_length_codelines call not inside it, but where this func is called with with_lenght=True. This simplifies things, as on the call site you see exactly, that you generate line for lenght, then line for index.
Alternatively we could use a format string and pass the actual var name that will hold df's len to it as a second argument, i.e.

def df_index_codelines(self, length):
...
        func_lines += [f'  res_index = numpy.arange({length})']

@AlexanderKalistratov AlexanderKalistratov merged commit 4dd90c4 into IntelPython:master Feb 18, 2020
@densmirn densmirn deleted the feature/df_getitem_bool_arr branch June 9, 2020 12:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants