You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/04/28 19:19:40 UTC
[GitHub] [beam] ryanthompson591 opened a new pull request, #17496: [BEAM-13983] changed file name from sklearn_loader to sklearn_inference
ryanthompson591 opened a new pull request, #17496:
URL: https://github.com/apache/beam/pull/17496
changed file name from sklearn_loader to sklearn_inference
------------------------
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
- [ ] [**Choose reviewer(s)**](https://beam.apache.org/contribute/#make-your-change) and mention them in a comment (`R: @username`).
- [ ] Format the pull request title like `[BEAM-XXX] Fixes bug in ApproximateQuantiles`, where you replace `BEAM-XXX` with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
- [ ] Update `CHANGES.md` with noteworthy changes.
- [ ] If this contribution is large, please file an Apache [Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
See the [Contributor Guide](https://beam.apache.org/contribute) for more tips on [how to make review process smoother](https://beam.apache.org/contribute/#make-reviewers-job-easier).
To check the build health, please visit [https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md](https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md)
GitHub Actions Tests Status (on master branch)
------------------------------------------------------------------------------------------------
[](https://github.com/apache/beam/actions?query=workflow%3A%22Build+python+source+distribution+and+wheels%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Python+Tests%22+branch%3Amaster+event%3Aschedule)
[](https://github.com/apache/beam/actions?query=workflow%3A%22Java+Tests%22+branch%3Amaster+event%3Aschedule)
See [CI.md](https://github.com/apache/beam/blob/master/CI.md) for more information about GitHub Actions CI.
--
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@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] ryanthompson591 commented on pull request #17496: [BEAM-13983] changed file name from sklearn_loader to sklearn_inference
Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on PR #17496:
URL: https://github.com/apache/beam/pull/17496#issuecomment-1112676641
@TheNeuralBit Originally, I felt the same. sklearn and pytorch, let me know if I'm wrong. The problem I was seeing was name collision.
For example if someone had the following imports.
import sklearn
from apache_beam.ml.inference import sklearn
there would be a name collision. Problem is that python does raise an exception here, but simply has the last import stand. So this will cause our users debugging head aches.
Instead they would need to do something like.
import sklearn
import apache_beam.ml.inference.sklearn as apache_sklearn (or sklearn_inference or whatever)
Our feeling is that users will often also import sklearn to use some of the libraries there. So in order to avoid error, we plan to use framework_inference as the name. I liked loader and andy liked inference. So we flipped a coin.
--
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@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] yeandy commented on pull request #17496: [BEAM-13983] changed file name from sklearn_loader to sklearn_inference
Posted by GitBox <gi...@apache.org>.
yeandy commented on PR #17496:
URL: https://github.com/apache/beam/pull/17496#issuecomment-1113677455
I'm ok with `apache_beam.ml.sklearn_inference` (or keeping as is). But I'm wondering if, in the future, there will be other ML related apis --not related to RunInference -- that we would develop. For example, we would want to have `ml/inference` and `ml/training` to minimize confusion. But we can refactor if/when that arises.
--
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@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17496: [BEAM-13983] changed file name from sklearn_loader to sklearn_inference
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17496:
URL: https://github.com/apache/beam/pull/17496#issuecomment-1112585296
Can one of the admins verify this patch?
--
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@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17496: [BEAM-13983] changed file name from sklearn_loader to sklearn_inference
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17496:
URL: https://github.com/apache/beam/pull/17496#issuecomment-1112585295
Can one of the admins verify this patch?
--
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@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17496: [BEAM-13983] changed file name from sklearn_loader to sklearn_inference
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17496:
URL: https://github.com/apache/beam/pull/17496#issuecomment-1112586142
Can one of the admins verify this patch?
--
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@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] yeandy commented on pull request #17496: [BEAM-13983] changed file name from sklearn_loader to sklearn_inference
Posted by GitBox <gi...@apache.org>.
yeandy commented on PR #17496:
URL: https://github.com/apache/beam/pull/17496#issuecomment-1112600173
We have plans to change `pytorch` to `pytorch_inference`
--
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@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] codecov[bot] commented on pull request #17496: [BEAM-13983] changed file name from sklearn_loader to sklearn_inference
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #17496:
URL: https://github.com/apache/beam/pull/17496#issuecomment-1112606209
# [Codecov](https://codecov.io/gh/apache/beam/pull/17496?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17496](https://codecov.io/gh/apache/beam/pull/17496?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7407dd7) into [master](https://codecov.io/gh/apache/beam/commit/f687a2d71f65cd53bf88f617f6c8c86e4fb73203?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f687a2d) will **increase** coverage by `0.00%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #17496 +/- ##
=======================================
Coverage 73.83% 73.84%
=======================================
Files 690 690
Lines 90830 90830
=======================================
+ Hits 67063 67071 +8
+ Misses 22558 22550 -8
Partials 1209 1209
```
| Flag | Coverage Δ | |
|---|---|---|
| python | `83.66% <ø> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/beam/pull/17496?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...thon/apache\_beam/ml/inference/sklearn\_inference.py](https://codecov.io/gh/apache/beam/pull/17496/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vbWwvaW5mZXJlbmNlL3NrbGVhcm5faW5mZXJlbmNlLnB5) | `92.68% <ø> (ø)` | |
| [...hon/apache\_beam/runners/worker/bundle\_processor.py](https://codecov.io/gh/apache/beam/pull/17496/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvYnVuZGxlX3Byb2Nlc3Nvci5weQ==) | `93.51% <0.00%> (+0.12%)` | :arrow_up: |
| [...ks/python/apache\_beam/runners/worker/sdk\_worker.py](https://codecov.io/gh/apache/beam/pull/17496/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy93b3JrZXIvc2RrX3dvcmtlci5weQ==) | `89.38% <0.00%> (+0.31%)` | :arrow_up: |
| [...eam/runners/portability/fn\_api\_runner/execution.py](https://codecov.io/gh/apache/beam/pull/17496/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vcnVubmVycy9wb3J0YWJpbGl0eS9mbl9hcGlfcnVubmVyL2V4ZWN1dGlvbi5weQ==) | `93.08% <0.00%> (+0.64%)` | :arrow_up: |
| [sdks/python/apache\_beam/utils/interactive\_utils.py](https://codecov.io/gh/apache/beam/pull/17496/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2Rrcy9weXRob24vYXBhY2hlX2JlYW0vdXRpbHMvaW50ZXJhY3RpdmVfdXRpbHMucHk=) | `95.12% <0.00%> (+2.43%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/beam/pull/17496?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/beam/pull/17496?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [f687a2d...7407dd7](https://codecov.io/gh/apache/beam/pull/17496?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit merged pull request #17496: [BEAM-13983] changed file name from sklearn_loader to sklearn_inference
Posted by GitBox <gi...@apache.org>.
TheNeuralBit merged PR #17496:
URL: https://github.com/apache/beam/pull/17496
--
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@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17496: [BEAM-13983] changed file name from sklearn_loader to sklearn_inference
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17496:
URL: https://github.com/apache/beam/pull/17496#issuecomment-1112586145
Can one of the admins verify this patch?
--
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@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] asf-ci commented on pull request #17496: [BEAM-13983] changed file name from sklearn_loader to sklearn_inference
Posted by GitBox <gi...@apache.org>.
asf-ci commented on PR #17496:
URL: https://github.com/apache/beam/pull/17496#issuecomment-1112586146
Can one of the admins verify this patch?
--
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@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] ryanthompson591 commented on pull request #17496: [BEAM-13983] changed file name from sklearn_loader to sklearn_inference
Posted by GitBox <gi...@apache.org>.
ryanthompson591 commented on PR #17496:
URL: https://github.com/apache/beam/pull/17496#issuecomment-1112571522
R: @yeandy
R: @TheNeuralBit
--
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@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] TheNeuralBit commented on pull request #17496: [BEAM-13983] changed file name from sklearn_loader to sklearn_inference
Posted by GitBox <gi...@apache.org>.
TheNeuralBit commented on PR #17496:
URL: https://github.com/apache/beam/pull/17496#issuecomment-1112711287
Thanks for the explanation! That makes sense, I didn't realize that Python wouldn't error in the event of a collision.
I might avoid the duplication of "inference" and just have an ml package with {module}_inference then, e.g. `apache_beam.ml.sklearn_inference`
--
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@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org