Skip to content

Deprecate Categorical vs argument#647

Merged
neerajprad merged 1 commit intodevfrom
drop-categorical-vs
Dec 21, 2017
Merged

Deprecate Categorical vs argument#647
neerajprad merged 1 commit intodevfrom
drop-categorical-vs

Conversation

@fritzo
Copy link
Copy Markdown
Member

@fritzo fritzo commented Dec 21, 2017

Closes #646
Blocking #606

This deprecates the vs argument to Categorical:

  • Removes vs from tests (so we can swap in torch.distributions.Categorical)
  • Adds a UserWarning indicating that vs is deprecated. (In general UserWarnings are preferred over DeprecationWarnings because Python 2.7 silences DeprecationWarning by default whereas Python 3.x does not silence them).

Note that the implementation has been left in place. We will remove the old implementation when moving to PyTorch distributions.

self.vs = self._process_data(vs)
self.log_pdf_mask = log_pdf_mask
if vs is not None:
warnings.warn('Categorical vs argument is deprecated', UserWarning)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are there existing examples/tutorials that use this argument currently? If not, we might as well just remove it on dev, as it will be removed by the next release.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I believe vs is not used in any examples or tutorials. I looked into removing vs code but that turned out to be more work than adding a simple warning 😄

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good! We can definitely get this merged as is, and remove it completely along with the tests soon.

@fritzo
Copy link
Copy Markdown
Member Author

fritzo commented Dec 21, 2017

@neerajprad ready to merge

@neerajprad neerajprad merged commit 4b15e6b into dev Dec 21, 2017
@fritzo fritzo deleted the drop-categorical-vs branch January 5, 2018 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants