Skip to content

Conversation

wch
Copy link
Collaborator

@wch wch commented Nov 27, 2023

This closes #814.

Many users have wondered why reactive.Value, .Calc, and .Effect are capitalized. There are technical reasons for this, but from a user perspective it makes more sense to just use lowercase names.

This PR changes those names to lowercase, and adds aliases so that the old capitalized names still work.

This PR does not change any examples because if users on a previous version of shiny were to use the example code, it wouldn't work for them, and it would be unclear to them why. In a future release, we should update the examples to use the new lowercase names.

@wch wch requested a review from schloerke November 27, 2023 21:16
@jcheng5
Copy link
Collaborator

jcheng5 commented Nov 29, 2023

I think we have consensus that @reactive.calc and @reactive.effect, as decorators, should be lower-cased.

I'm a little surprised to see reactive.Value() being lower-cased to reactive.value(), as that's just a plain old constructor?

@jcheng5
Copy link
Collaborator

jcheng5 commented Nov 29, 2023

And IF we do really want reactive.value to be lower-cased, I think we should still have class Value and then create a value alias (or even make a wrapper function def value(x: T) -> reactive.Value[T]) so that it still makes sense to have reactive.Value for type annotations.

@schloerke
Copy link
Collaborator

Oooooo. I really like the small alias functions.

  • Implement lowercase names as as alias functions that return Uppercase class objects

@wch
Copy link
Collaborator Author

wch commented Nov 29, 2023

I think we should provide a lowercase version of value, because if we don't, I think we'll get the same questions about "why is this thing capitalized?" But I don't feel strongly about it.

I think it makes sense to keep the Value class, and then make value an alias to it.

So in short, I can roll back the Value->value changes, and then just add value as an alias. Then if we decide we don't like the alias, we can take it out, and it'll be just a one-line change.

@wch wch merged commit e2177a8 into main Dec 1, 2023
@wch wch deleted the reactive-case branch December 1, 2023 23:30
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.

Should add lowercase aliases for reactive.value, reactive.calc, reactive.effect
3 participants