You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/03/08 12:38:09 UTC
[GitHub] [arrow-datafusion] alamb opened a new pull request, #5515: Minor: Improve docs for UserDefinedLogicalNode `dyn_eq` and `dyn_hash`
alamb opened a new pull request, #5515:
URL: https://github.com/apache/arrow-datafusion/pull/5515
# Which issue does this PR close?
Related to https://github.com/apache/arrow-datafusion/issues/5400
# Rationale for this change
These new required methods were aded https://github.com/apache/arrow-datafusion/pull/5421 /
and I felt a little more documentation could help people during upgrade.
# What changes are included in this PR?
Add doc comments and examples
# Are these changes tested?
Yes, they are doc tests and thus tested via `cargo test --doc` and `cargo test --all`
# Are there any user-facing changes?
More docs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb merged pull request #5515: Minor: Improve docs for UserDefinedLogicalNode `dyn_eq` and `dyn_hash`
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #5515:
URL: https://github.com/apache/arrow-datafusion/pull/5515
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] ursabot commented on pull request #5515: Minor: Improve docs for UserDefinedLogicalNode `dyn_eq` and `dyn_hash`
Posted by "ursabot (via GitHub)" <gi...@apache.org>.
ursabot commented on PR #5515:
URL: https://github.com/apache/arrow-datafusion/pull/5515#issuecomment-1460816974
Benchmark runs are scheduled for baseline = 89ca58326f5cc3901c33b522ff3848af47ebf4b2 and contender = 0ead640e887c19607a371c22a183dc91a55baf45. 0ead640e887c19607a371c22a183dc91a55baf45 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/da7e41a210a241c3a4eb767351ab1e62...d3a4b0f5726940cabd26d9b7f7d230ca/)
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] [test-mac-arm](https://conbench.ursa.dev/compare/runs/a664f46f9fd4480b84e461b67d2bfd33...32f904b863fc4e3e8329168d33ca99fd/)
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/75b80b6f50f9442a96b87727ee6ce783...ad7aee88ec9d4e63964a23f1211338a9/)
[Skipped :warning: Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/651da484f804462f9f83da34d9825a0a...7a955df7ffe74ef5b3967f67ebb17860/)
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #5515: Minor: Improve docs for UserDefinedLogicalNode `dyn_eq` and `dyn_hash`
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #5515:
URL: https://github.com/apache/arrow-datafusion/pull/5515#issuecomment-1460352820
> @alamb What do you think about the idea? 💡
It seems like a good idea, but I think I would need to see an example of what an existing UserDefinedNode would look like. Could you make a draft PR with the idea?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] alamb commented on pull request #5515: Minor: Improve docs for UserDefinedLogicalNode `dyn_eq` and `dyn_hash`
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #5515:
URL: https://github.com/apache/arrow-datafusion/pull/5515#issuecomment-1460814447
I think these docs make main better than it was before so I am going to merge. It would be great if @mslapek 's work makes them unecessary 🙏
BTW @mslapek I think the next datafusion release will be made in 2 days time (Friday) so if we are going to propose a change to the API it would be nice to do it before then, to avoid users having to deal with two API changes in quick succession
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow-datafusion] mslapek commented on pull request #5515: Minor: Improve docs for UserDefinedLogicalNode `dyn_eq` and `dyn_hash`
Posted by "mslapek (via GitHub)" <gi...@apache.org>.
mslapek commented on PR #5515:
URL: https://github.com/apache/arrow-datafusion/pull/5515#issuecomment-1460130436
@alamb These docs were really missing. This PR is great.
TBH I was considering to make (non-object safe) trait `UserDefinedLogicalNodeCore: Eq, Hash` (notice **Core**) and add a blanket implementation `UserDefinedLogicalNodeCore` -> `UserDefinedLogicalNode`.
This would avoid the mentioned boilerplate (`as_any`, `dyn_eq`, `dyn_hash`) from the implementer side.
@alamb What do you think about the idea? 💡
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org