Skip to content

Conversation

@kmsquire
Copy link
Member

  • Dict and OrderedDict constructors have changed, and
    DefaultDict/DefaultOrderedDict haven't kept up.

  • The changes:

    Dict(KeyType, ValType) => Dict{KeyType, ValType}()
    Dict(ks, vs) is deprectated

  • This deprecates working versions of these from DefaultDict
    and friends, and fixes broken calls which attempt to
    use these old constructors.

* Dict and OrderedDict constructors have changed, and
  DefaultDict/DefaultOrderedDict haven't kept up.
* The changes:

   Dict(KeyType, ValType) => Dict{KeyType, ValType}()
   Dict(ks, vs) is deprectated

* This deprecates working versions of these from DefaultDict
  and friends, and fixes broken calls which attempts to
  use these old constructors.
@codecov-io
Copy link

codecov-io commented Nov 21, 2016

Current coverage is 92.92% (diff: 100%)

Merging #233 into master will increase coverage by 0.70%

@@             master       #233   diff @@
==========================================
  Files            29         29          
  Lines          2351       2346     -5   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           2168       2180    +12   
+ Misses          183        166    -17   
  Partials          0          0          

Powered by Codecov. Last update af2c97e...03f14c0

@kmsquire kmsquire changed the title Deprecate or fix old/broken DefaultDict constructors RFC: Deprecate or fix old/broken DefaultDict constructors Nov 21, 2016
@kmsquire
Copy link
Member Author

kmsquire commented Nov 21, 2016

This PR should be ready for review. Linux builds all passed, osx builds are still waiting to run. (We need to enable AppVeyor at some point... @tkelman, might you help with that?)

The functional changes shouldn't be controversial (just updating the interfaces), but a quick review would still be welcome.
For the deprecated functions, I arbitrarily assigned a Julia version in which to remove the deprecations. Any thoughts (👍 or 👎 ) would be welcome.

cc: @DanielArndt @rawls238

@kmsquire kmsquire mentioned this pull request Nov 21, 2016
@kmsquire kmsquire force-pushed the kms/update_default_dict_constructors branch from 81b0165 to 2018d61 Compare November 21, 2016 20:26
…structors

* Adding tests revealed some issues with the constructors.  Tests are
  good!
@kmsquire kmsquire force-pushed the kms/update_default_dict_constructors branch from 2018d61 to 03f14c0 Compare November 21, 2016 21:04
@kmsquire
Copy link
Member Author

Everything passes. Will merge tomorrow if there are no comments.

@tkelman
Copy link
Contributor

tkelman commented Nov 21, 2016

should mostly just be adding a yml file to enable appveyor, though may need to manually set the webhook if something goes wrong with enabling the repo from the appveyor ui.

@rawls238
Copy link
Contributor

LGTM

@kmsquire kmsquire merged commit f21aad1 into master Nov 22, 2016
@kmsquire kmsquire deleted the kms/update_default_dict_constructors branch November 22, 2016 17:28
@kmsquire
Copy link
Member Author

Thanks @rawls238

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.

5 participants