Updated README and added an optional argument #58
Conversation
I have updated the README.md file for v1.0.5, which mentions that the user can also use their personal API keys. Also, added an optional parameter 'length' in the get_random_word() and get_random_words() functions which reduces the function call's verbosity when a word of an certain length is required.
|
Please do review this pull request and tell me the changes I need to incorporate in order to pass all the tests. I really haven't changed it a whole lot. So, I don't know what's making it fail. |
| @@ -57,14 +64,15 @@ r.word_of_the_day() | |||
| - `maxDictionaryCount (integer)` - Maximum dictionary count (optional) | |||
| - `minLength (integer)` - Minimum word length (optional) | |||
| - `maxLength (integer)` - Maximum word length (optional) | |||
| - `length (integer)` - Exact word length (optional) | |||
There was a problem hiding this comment.
This parameter doesn't comply with API spec provided by wordnik API. I think if any dev wanted to get specific word by length, then one can use minLength = maxLength while calling the API.
Plus, I am planning to extend this library by providing various sources, so we can add this optional parameter if providing random word using local dictionary. I still have to do research how, we can pull word from local dictionary present in OS though
There was a problem hiding this comment.
I understand that the Wordnik API doesn't support the parameter length. But as mentioned in Issue #22 by Erik Boesen, the function call for a word of a specific length is verbose and inefficient. So, what I've done is, I've accepted the optional parameter length from the user, set the max_length and min_length parameters internally to the value given in the parameter by the user and then deleted the length parameter from the kwargs before passing it to the other functions for exception handling. So basically, my change reduces unnecessary verbosity at the user's end because otherwise if I wanted to get a 5 letter word, my call would have to be this
rw.get_random_word(min_length=5, max_length=5)
Now, from my commit, it would change to
rw.get_random_word(length=5)
This is both logical and more efficient for the user calling the function.
It's not your fault, as to run tests, I had encrypted API key and committed to the git. Every time, a build is triggered SECRET key which I had added in the repository get accessed and decrypt the file. But there is an exception that SECRET Key can not be passed to the build triggered from forked repositories. I would fix this probably later, or if you want to hack into it than a PR is welcome. |
So, how do I fix this? Can we not create a feature branch on this repository and give me access to that branch alone, so that it would be easier? Also, if all forked repos PR's are failing builds, isn't that a bad thing? Because no where does it say that the builds would definitely fail and I never even knew until you told me. |
I have updated the README.md file for v1.0.5, which mentions that the user can also use their personal API keys. Also, added an optional parameter 'length' in the get_random_word() and get_random_words() functions which reduces the function call's verbosity when a word of an certain length is required.
This solves three issues in total #49 #22