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 #12451 by providing a better error message #12453

Merged
merged 1 commit into from
Aug 12, 2015

Conversation

rened
Copy link
Member

@rened rened commented Aug 4, 2015

No description provided.

@rened rened force-pushed the dict branch 3 times, most recently from ca58ab2 to ac872fd Compare August 4, 2015 15:54
@mbauman
Copy link
Member

mbauman commented Aug 4, 2015

Instead of fixing up the thrown error with a try/catch block (which is fairly expensive), it'd be better to detect the error state itself before it occurs.

I'm not sure that you can do much about non-iterables (that's probably solved by a more informative error display at the REPL for missing start methods), but you could ensure that each element is itself an iterable collection of 2 elements instead of using automatic destructing in the for loop.

@rened
Copy link
Member Author

rened commented Aug 4, 2015

@mbauman try/catch: true - my rationale was that running that check every time a Dict is constructed is also expensive (e.g. for many elements), so rather only check on error.

So better check for start/next/done first, then construct and let it fail hard for non-iterators? I'll check a few versions for speed.

@mbauman
Copy link
Member

mbauman commented Aug 4, 2015

A few performance spot-checks would be ideal. At the very least, you can move this into the catchall dict_with_eltype (or inner constructor) for collections that aren't a bunch of Tuples or Pairs, since those won't have this problem.

@tkelman tkelman added the error handling Handling of exceptions by Julia or the user label Aug 5, 2015
StefanKarpinski added a commit that referenced this pull request Aug 12, 2015
fix #12451 by providing a better error message
@StefanKarpinski StefanKarpinski merged commit 90fe14d into JuliaLang:master Aug 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error handling Handling of exceptions by Julia or the user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants