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

Renaming decondition to uncondition #112

Closed
wants to merge 1 commit into from
Closed

Conversation

sunxd3
Copy link
Member

@sunxd3 sunxd3 commented Dec 17, 2024

fix #84

@sunxd3
Copy link
Member Author

sunxd3 commented Dec 17, 2024

@penelopeysm this discussion might be before you join, curious for your thoughts on this issue

@penelopeysm
Copy link
Member

I'm kind of on the fence about this, I get that the naming is more consistent with unfix but it doesn't feel like a good enough reason to make a breaking change.

If we go ahead with this, let's at least make sure that we have a plan for percolating this up to DynamicPPL and Turing. Are we going to keep a deprecated implementation of decondition around like this?

Base.@deprecate decondition(model) uncondition(model)

@sunxd3
Copy link
Member Author

sunxd3 commented Dec 18, 2024

On second thought, I like decondition better than uncondition. The reason is de- means "removal", but "un-" doesn't. Also Penny made a good point here, I am not sure the naming consistency is really worth it for a: (1) arguably worse name (2) breaking release from Turing&DPPL.

@yebai I would suggest just close the issue instead.

@yebai yebai closed this Dec 18, 2024
@yebai yebai deleted the sunxd/rename_decondition branch December 18, 2024 11:49
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.

Rename decondition to uncondition
3 participants