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

make broadcasting into getproperty extensible #39473

Merged
merged 4 commits into from
Mar 12, 2021
Merged

Conversation

simeonschaub
Copy link
Member

Don't know if dotgetproperty is a good name for this, since as discussed in #36741, the way this works is a bit different from dotview. Right now, dotproperty returns a function which is used instead of getproperty, but not sure if that is a bit strange and we should just make it behave like dotview instead.

fixes #36741

Don't know if `dotgetproperty` is a good name for this, since as discussed in #36741, the way this works is a bit different from `dotview`. Right now, `dotproperty` returns a function which is used instead of `getproperty`, but not sure if that is a bit strange and we should just make it behave like `dotview` instead.

fixes #36741
@simeonschaub simeonschaub added compiler:lowering Syntax lowering (compiler front end, 2nd stage) broadcast Applying a function over a collection labels Feb 1, 2021
@simeonschaub simeonschaub added the triage This should be discussed on a triage call label Feb 4, 2021
@JeffBezanson
Copy link
Member

I'm inclined to have it do the operation (i.e. dotgetproperty(x,y) = getproperty(x,y)) instead of returning a function, if at all possible. It just seems easier to understand.

@simeonschaub
Copy link
Member Author

Yes, that's probably less awkward.

@simeonschaub
Copy link
Member Author

@JeffBezanson Is this good to go?

@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label Mar 11, 2021
@JeffBezanson JeffBezanson added the merge me PR is reviewed. Merge when all tests are passing label Mar 11, 2021
@vtjnash vtjnash added merge me PR is reviewed. Merge when all tests are passing and removed merge me PR is reviewed. Merge when all tests are passing labels Mar 11, 2021
@simeonschaub simeonschaub merged commit c9c1d3e into master Mar 12, 2021
@simeonschaub simeonschaub deleted the sds/dotgetproperty branch March 12, 2021 08:03
@bkamins
Copy link
Member

bkamins commented Mar 12, 2021

Would it be possible to have it in 1.6 (or in general in new target Julia LTS release). It would be great for DataFrames.jl users to be able to assume this functionality is always available (otherwise I would need to write in the documentation that some basic functionality like df.a .= 1 is available only in Julia 1.7 and upwards which is OK, but might be confusing for the users).

@KristofferC
Copy link
Member

Feature freeze was a long time ago, so don't think any new features can go into 1.6.

@simeonschaub simeonschaub removed the merge me PR is reviewed. Merge when all tests are passing label Apr 3, 2021
ElOceanografo pushed a commit to ElOceanografo/julia that referenced this pull request May 4, 2021
* make broadcasting into getproperty extensible

Don't know if `dotgetproperty` is a good name for this, since as discussed in JuliaLang#36741, the way this works is a bit different from `dotview`. Right now, `dotproperty` returns a function which is used instead of `getproperty`, but not sure if that is a bit strange and we should just make it behave like `dotview` instead.

fixes JuliaLang#36741

* make dotgetproperty call getproperty directly

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request May 9, 2021
* make broadcasting into getproperty extensible

Don't know if `dotgetproperty` is a good name for this, since as discussed in JuliaLang#36741, the way this works is a bit different from `dotview`. Right now, `dotproperty` returns a function which is used instead of `getproperty`, but not sure if that is a bit strange and we should just make it behave like `dotview` instead.

fixes JuliaLang#36741

* make dotgetproperty call getproperty directly

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
broadcast Applying a function over a collection compiler:lowering Syntax lowering (compiler front end, 2nd stage)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow x.y .= z not to use getproperty(x, y)
5 participants