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

Add @cython.except_ #1653

Closed
wants to merge 2 commits into from
Closed

Add @cython.except_ #1653

wants to merge 2 commits into from

Conversation

antocuni
Copy link

This PR adds a new directive to specify the "except" value for cdef/cpdef functions when using the pure python mode.

I don't know much about the Cython internals, so in case there is a better/cleaner way to do it, please comment :)

@scoder
Copy link
Contributor

scoder commented Apr 9, 2017

This is incomplete because it does not cover all functionality: except X, except? X, except *. If we add support for this, we should support all three.

@antocuni
Copy link
Author

Yes, I agree that it's incomplete. I opened the PR mostly for getting feeback before doing the rest of the work :). For the '?' case, I think there are at least two possible options for which syntax to use:

  1. @cython.except_(-1, '?')
  2. @cython.except_('?', -1)

Moreover, for the '*' case:
3. @cython.except_(None, '*')
4. @cython.except_('*')

Option 1 is more regular, because the '?' or '*' is just an optional argument whose default value can be thought as None or ''. Option 2 is closer to the syntax we use in pyx files, but it makes the signature very irregular. Option 3 is ugly, but it is the only choice we have in case we decide to use a "regular" signature for the decorator. Option 4 plays very well with option 2.

Another alternative is to do everything in @cython.returns:
5. @cython.returns(long, except_=-1)

However, it makes it harder to come up with a reasonable syntax for the '?' and '*' cases; some ideas:
6. @cython.returns(long, except_=-1, except_mode='?')
7. @cython.returns(long, except_maybe=-1)
8. @cython.returns(long, except_=('?', -1))
9. @cython.returns(long, except_='?-1') # this is uguly

@antocuni
Copy link
Author

antocuni commented Jun 6, 2017

sorry for bringing this up again. So, which syntax do you like more?

@TeamSpen210
Copy link
Contributor

How about the signature except_(type=None, *, check=False)? This would provide the following matches:

  • except -1: @cython.except_(-1)
  • except? -1: @cython.except_(-1, check=True)
  • except *: @cython.except_(check=True)

No arguments or both default could do nothing at all. For Python 2.7 support, check could remain a positional argument (or *args, **kwargs could be used manually).

@antocuni
Copy link
Author

antocuni commented Jun 7, 2017

good hint, I like this syntax. @scoder what do you think?

@TeamSpen210
Copy link
Contributor

Actually now I think about it, Py2 won't be a problem since shadow uses *args, **kwargs for everything anyway and the compiler does its own parsing.

@scoder
Copy link
Contributor

scoder commented Sep 1, 2017

Since this is only needed when @returns() is already used, I think merging it into that is a good idea. And I like the keyword argument signature with check=True, so I vote for

@returns(ctype, *, except_=None, check=False)

That nicely covers all four cases.

Regarding the except_ keyword, I do see the advantage of using the same word as for the except signature declaration, but requiring an underscore feels ugly. I fail to find a better alternative, though. exception=-1 doesn't feel right since it's not an exception, except_on and the like are arbitrary new things to remember, except_returns is redundant. So, for now, except_ seems as good as it gets.

@scoder
Copy link
Contributor

scoder commented Sep 3, 2017

Thinking about this some more, it feels like people might want to declare the return type via annotations in the future (def f() -> cython.long:), in which case requiring them to additionally use @returns would be redundant. I think that brings us back to the @except_() decorator then.

@scoder scoder closed this in c372d2d Sep 3, 2017
@scoder
Copy link
Contributor

scoder commented Sep 3, 2017

I've opted for the name @cython.exceptval() in the end, because I didn't like the underscore. See c372d2d.

@antocuni
Copy link
Author

antocuni commented Sep 4, 2017

thank you @scoder :)
Do you have a rough estimate about the date of the next release?

@scoder
Copy link
Contributor

scoder commented Sep 4, 2017

Current master has some rather big new features, so I'd like to get some testing on them first. But looking back, getting 0.26 out took way too long, so I'd like to have 0.27 ready within weeks rather than months. That's as close to an estimate as it gets. ;)

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

3 participants