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

turn deprecation warnings into errors #585

Closed
corakingdon opened this issue Nov 1, 2019 · 5 comments
Closed

turn deprecation warnings into errors #585

corakingdon opened this issue Nov 1, 2019 · 5 comments
Assignees

Comments

@corakingdon
Copy link
Collaborator

corakingdon commented Nov 1, 2019

Steps:

  1. Release v0.10.0 with the following depreciation warnings (using either @deprecate or Base.depwarn
  2. Release v1.0.0 where these now error, mostly with a MethodException since methods will be removed. This will require cleaning out the deprecated code, any helper functions for deprecation warnings, and any tests of the now removed code.

Deprecations include the following (also search for Base.depwarn and @deprecate)

  • Integer indexing into a TimestepArray at the time index position with getindex or setindex!
  • using the functions is_time or is_timestep
  • indexing into a simulation instance with square brackets
  • changes that need to be made to set_param!
  • type parameterization in @Defcomp New syntax in defcomp for datum type parameterization #638
  • renaming marginal to modified in MarginalModel's
  • change replace_comp! to replace! w/ julia argument convention (use pair)
@corakingdon corakingdon added this to the v1.0.0 milestone Nov 1, 2019
@lrennels
Copy link
Collaborator

lrennels commented Nov 1, 2019

I think it's probably a good idea to add deprecation warnings to anything where it's a somewhat one to one change with functions, and then the rest of the things could be in a nice comprehensive list or something that we can post on the website/docs.

@lrennels
Copy link
Collaborator

lrennels commented Nov 1, 2019

In terms of indexing into a simulation index with square brackets, I have completely removed that getindex function and replaced it with a getdataframe, so we'd have to add the getindex back in if we wanted to deprecate it. I have a feeling no one is using that so it might not be necessary but either way would be fine.

@rjplevin
Copy link
Collaborator

rjplevin commented Nov 1, 2019

If nobody is using it, the deprecation does no harm. If they are using it and you don't deprecate, it causes grief. Any reason to not deprecate? Seems like the better choice to me!

@lrennels
Copy link
Collaborator

lrennels commented Nov 1, 2019

Agreed, no reason not to deprecate except laziness, I'll go grab that old PR and add back in a deprecated getindex function!

@lrennels lrennels modified the milestones: v0.10.0, v1.0.0 Mar 13, 2020
@lrennels
Copy link
Collaborator

lrennels commented Jun 13, 2020

#713

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

No branches or pull requests

3 participants