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

Improve clr.accepts/returns #1449

Merged
merged 4 commits into from
May 11, 2022
Merged

Conversation

BCSharp
Copy link
Member

@BCSharp BCSharp commented May 11, 2022

Laggard from #52.

Copy link
Contributor

@slozier slozier left a comment

Choose a reason for hiding this comment

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

Seems fine to me. Too bad the Checker classes are public - seems like unnecessary API surface (unless there's some use case I'm not thinking of).

@BCSharp
Copy link
Member Author

BCSharp commented May 11, 2022

Too bad the Checker classes are public - seems like unnecessary API surface (unless there's some use case I'm not thinking of).

I had the same reaction. I consider this part of API "legacy". It is rather limited in scope and was buggy so I expect it is not widely used. Going forward, rather than putting more effort into it, I'd rather see a clr.assertTypeAnnotations decorator.

@BCSharp BCSharp merged commit b16734a into IronLanguages:master May 11, 2022
@BCSharp BCSharp deleted the arg_ret_checkers branch May 11, 2022 21:55
@BCSharp
Copy link
Member Author

BCSharp commented May 27, 2022

@slozier, I've been thinking: maybe the moment before the final 3.4 release is a good opportunity to remove the Checker classes from public API after all? A migration from 2 to 3 is about breaking changes (though I think it is unlikely this would break anyone's existing code).

@slozier
Copy link
Contributor

slozier commented May 27, 2022

Fine by me. If someone really needs them they can file an issue.

@BCSharp
Copy link
Member Author

BCSharp commented May 28, 2022

It turns out those types have to be public in order to be callable from Python. I'm parking the issue.

@slozier
Copy link
Contributor

slozier commented May 28, 2022

It turns out those types have to be public in order to be callable from Python. I'm parking the issue.

Hmm, we could make them PythonHidden (and internal constructors maybe) so they don't show up on clr? Anyway, no big deal.

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