You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by giwa <gi...@git.apache.org> on 2014/05/18 12:36:14 UTC
[GitHub] spark pull request: Consistency for function naming in Duration
GitHub user giwa opened a pull request:
https://github.com/apache/spark/pull/814
Consistency for function naming in Duration
toFormattedString should represent formatted millisecond like "10 ms" not simply give back "10"
toString should represent string of duration. It should simply give back string of millisecond.
Currently like this.
duration = Duration(10)
duration.toString()
>> "10 ms"
duration.toFormattedString()
>> "10"
Should be
duration = Duration(10)
duration.toString()
>> "10"
duration.toFormattedString()
>> "10 ms"
Please explain what does "formatted" mean? Why does it simply give millisecond with string format?
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/giwa/spark patch-1
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/814.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #814
----
commit 9db6ae1a74f5aa05dfa1ccd9550f608c235257e1
Author: Ken Takagiwa <ug...@gmail.com>
Date: 2014-05-18T10:34:30Z
Consistency for naming for Duration
toFormattedString should represent formatted millisecond like "10 ms" not simply give back "10"
toString should represent string of duration. It should simply give back string of millisecond.
Currently like this.
duration = Duration(10)
duration.toString()
>> "10 ms"
duration.toFormattedString()
>> "10"
Should be
duration = Duration(10)
duration.toString()
>> "10"
duration.toFormattedString()
>> "10 ms"
Please explain what does "formatted" mean? Why does it simply give milli second with string foramt
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Consistency for function naming in Duration
Posted by ash211 <gi...@git.apache.org>.
Github user ash211 commented on the pull request:
https://github.com/apache/spark/pull/814#issuecomment-43446583
Where else in the Spark codebase are these methods called? If we're switching their meaning we need to make sure that callers are updated to expect the new formats.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Consistency for function naming in Duration
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/814#issuecomment-43436522
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Consistency for function naming in Duration
Posted by giwa <gi...@git.apache.org>.
Github user giwa closed the pull request at:
https://github.com/apache/spark/pull/814
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Consistency for function naming in Duration
Posted by giwa <gi...@git.apache.org>.
Github user giwa commented on the pull request:
https://github.com/apache/spark/pull/814#issuecomment-43447806
@ash211 Thank you for your comment. After my second thought, this suggestion is not good. toString in Java world gives back human readable format.
The reason why I came up this question is I am writing wrapper of Duration in Python. Since you are familiar with Python, Could I ask some questions?
Do we need dunder str and dunder repr in Duration? If they are needed, what they should give back respectively?
```
str(Duration)
"10 ms"
repr(Duration)
"10"
```
BTW
toFormattedString is not used anywhere. Regarding to toString, I think it is not used anywhere in streaming, though I need more time to look deeply.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Consistency for function naming in Duration
Posted by ash211 <gi...@git.apache.org>.
Github user ash211 commented on the pull request:
https://github.com/apache/spark/pull/814#issuecomment-43462755
I'm familiar with the Python language but much less so the conventions like str vs repr.
From this SO post though, it looks like __str__ should be "10 ms", but also __repr__ should be something like "Duration(10ms)"
http://stackoverflow.com/questions/1436703/difference-between-str-and-repr-in-python
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---