Conversation
|
Hello @guifelix, Thanks for contributing, it looks great via the code! 👍 |
|
I guess the NLU improvement will help for that package (as Leon can extract date entities from the core and gives it to modules). You can take a look at this branch to have a better understanding. Or you can wait until I finish that improvement and once I wrote the proper documentation. |
|
I've edited the docs according to the NLU improvements. You can take a look here 😉 Let me know if you have any questions. |
theoludwig
left a comment
theoludwig
left a comment
There was a problem hiding this comment.
Thanks for your contribution! @guifelix
I have a new idea for this package, we could ask "What time is it in city_name" for example "What time is it in San Francisco ?", so we could know the time in multiple time zone.
I submit several comments about your code, feel free to discuss.
| requests = "==2.21.0" | ||
| pytube = "==9.2.2" | ||
| tinydb = "==3.9.0" | ||
| parsedatetime = "*" |
There was a problem hiding this comment.
I don't think, it is great to use "*" for the version number because things could break between releases, so it would be better to have an exact version number.
| ... | ||
| ``` | ||
|
|
||
| ## Todo |
There was a problem hiding this comment.
We should not use "Todo" inside markdown files, instead it would be GitHub issues or Trello Card.
What do you think?
| import utils | ||
| import datetime | ||
| import parsedatetime | ||
| # import dateparser |
| if word.startswith('the'): | ||
| word = word.replace('the', 'one', 1) | ||
|
|
||
| cal = parsedatetime.Calendar() |
There was a problem hiding this comment.
We should avoid abbreviations for variable names. So instead of cal, we could call it calendar.
| random = randint(0, 2) | ||
|
|
||
| # https://stackoverflow.com/questions/9647202/ordinal-numbers-replacement | ||
| suf = lambda n: "%d%s"%(n,{1:"st",2:"nd",3:"rd"}.get(n if n<20 else n%10,"th")) |
There was a problem hiding this comment.
It is quite hard to understand, what this is trying to do, maybe we could refactor this in multiple functions/method or with variables etc.
| #trying to use dateparser lib | ||
| # return utils.output('end', 'guess', utils.translate('guess', { 'guess': dateparser.parse(word) })) |
There was a problem hiding this comment.
We should avoid useless comments.
| hrs = time.hour | ||
| mins = time.minute | ||
| msg = "" |
There was a problem hiding this comment.
As I previously said, we should avoid abbreviations for variable names.
|
|
||
| def saycurrenttime(string): | ||
|
|
||
| # https://sukhbinder.wordpress.com/2013/12/29/time-in-words-with-python/ |
There was a problem hiding this comment.
I guess we don't need this link.
3 participants
What type of change does this PR introduce?
Does this PR introduce breaking changes?
List any relevant issue numbers:
Description:
A package deal with date and time