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 warning on unordered to ordered conversion. Fixes #216 #237

Merged
merged 2 commits into from
Jan 26, 2017

Conversation

cstjean
Copy link
Contributor

@cstjean cstjean commented Dec 7, 2016

I tried to match the tone from Base, but please let me know if anything sounds off.

@cstjean cstjean changed the title Add warning on unordered to ordered conversion Add warning on unordered to ordered conversion. Fixes #216 Dec 7, 2016
@codecov-io
Copy link

codecov-io commented Dec 7, 2016

Current coverage is 95.85% (diff: 71.42%)

Merging #237 into master will decrease coverage by 0.07%

@@             master       #237   diff @@
==========================================
  Files            30         30          
  Lines          2213       2220     +7   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           2123       2128     +5   
- Misses           90         92     +2   
  Partials          0          0          

Powered by Codecov. Last update 6846f65...eae3098

@kmsquire
Copy link
Member

@cstjean, sorry for the delay on this, but would you be able to add some tests?

@kmsquire
Copy link
Member

(Also, there is a minor conflict.)

@cstjean
Copy link
Contributor Author

cstjean commented Jan 25, 2017

Can I test the warnings? Or do you mean to make sure that is_ordered(Dict)==true?

@kmsquire
Copy link
Member

Just testing is_ordered (for all of the dictionaries for which it is defined) would be fine. Base runs julia with --depwarn=error, which could be caught, but I don't think packages are tested that way by default.

@cstjean
Copy link
Contributor Author

cstjean commented Jan 26, 2017

Tested and rebased.

@kmsquire
Copy link
Member

Thanks!

@kmsquire kmsquire merged commit 52b74de into JuliaCollections:master Jan 26, 2017
@kmsquire
Copy link
Member

For testing the deprecation see JuliaLang/julia#20348.

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.

3 participants