Skip to content

MAINT: Tidy macros in scalar_new #15441

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

Merged
merged 1 commit into from
Jan 27, 2020

Conversation

eric-wieser
Copy link
Member

Rather than trying to keep track of weird tri-state @default@ and @work@ variables, this uses a primitive dictionary with types as keys.


@seberg, is this the type of cleanup you had in mind in #15393 (review)

@eric-wieser eric-wieser requested a review from seberg January 27, 2020 00:32
#define _NPY_UNUSED1_0
#define _NPY_UNUSED1_1
#define _NPY_UNUSED1_2 NPY_UNUSED
#if @TYPE@_IS_OBJECT
Copy link
Member

Choose a reason for hiding this comment

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

I suppose these should be #if _@TYPE@_IS_OBJECT? (Was going to add defined first, but that is unnecessary I guess).
I think I like this style, although did not look through all yet. IIRC one thing that I was wondering if we cannot hardcode the result of PyTuple_GET_ITEM(cls.tp_bases, num)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch, thanks

Copy link
Member Author

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

Updated to use defined(), I think it's a little more understandable that way.

Rather than trying to keep track of weird tri-state `@default@` and `@work@` variables, this uses a primitive dictionary with types as keys.
@seberg
Copy link
Member

seberg commented Jan 27, 2020

Thanks, the dance around NPY_UNUSED is unfortunate, but I prefer this style, so putting it in.

@seberg seberg merged commit 7b71975 into numpy:master Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants