-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add docstring examples to each function #29
Comments
Hello, @ilayn I'd be down for this 😄 Although, as I'm not really well versed in Control Engineering (at the moment), I'd be glad if you could point out some easier functions I could work on for the beginning 😋 |
Hi @whzup so nice of you to offer a helping hand. You can start from anyone of them as you find interesting. I am really hoping that being well-versed is not a prerequisite for this library. Otherwise it means that I have failed. So please let me know if you think something does not feel right or cumbersome in general. I've tried to make an introductory tutorial under If you had the chance to tinker with the library you can add what you have done such as creating models or converting a I am using the rubrics of NumPy docstrings. As an example, you can add things to the last part of each docstring (before the references sections if any) as if you have typed them on the console. Take the example for least common polynomial as a template https://github.com/ilayn/harold/blob/master/harold/_polynomial_ops.py#L76-L87 and it is rendered like this documentation page Just a few examples sufficient at this point. We don't need to provide any tutorials (yet). Note that it is very possible that you might discover bugs or strange behavior since I've been blinded by my own design 😃 So, don't hesitate to slap me about my own stupidity. Clone the repository and create a branch via This also tells me that I should make a contribution guide. See, you have contributed already! When you are satisfied, you can send a PR with your additions. Let me know if you need help with the git and PR creation. It took me some time to figure it out too. Thanks again, |
@ilayn Thank you for this kind introduction!
So it would be possible to just copy the code from the Python Shell right? |
Yes exactly. You can also use a Jupyter notebook or Spyder but then reorganize the output as if it was coming from a shell. In other words you don't need to work on a Python shell. Later on when I find the time I will include the doctesting suite such that a robot will walk through the examples sections of the each docstring and run the examples automatically comparing what it got and what it should have got. Hence we stick to "Python shell" look and feel to be future-proof. |
@ilayn I just started to look around in the package and played around with the polynomial operations. I tried >>> a = np.array([2, 3, 5, 8])
>>> b = np.array([1, 3, 4])
>>> c = np.array([6, 9, 10, -8, 6])
>>> haroldpolyadd(a, b, c)
array([ 6., 11., 14., 0., 18.])
>>> d = np.array([-2, -4 ,3, -1])
>>> haroldpolyadd(a, b, d)
array([ 0., 0., 11., 11.])
>>> haroldpolyadd(a, b, d, trim_zeros=False)
array([ 0., 0., 11., 11.])
>>> haroldpolyadd(a, b, d, trim_zeros=True)
array([ 0., 0., 11., 11.]) |
Ah, yes. It is only meant for output trimming. The inputs are always trimmed. Example: >>> a = np.array([0, 0, 0])
>>> b = np.array([0, 0, 0, 1, 3, 4])
>>> c = np.array([0, 9, 10, -8, -4])
>>> haroldpolyadd(a, b, c, trim_zeros=False)
array([ 0., 0., 9., 11., -5., 0.])
>>> haroldpolyadd(a, b, c, trim_zeros=True)
array([ 9., 11., -5., 0.]) The reason for this optional argument is to make sure that the sizes don't change. So imagine you have two arrays of the same size, say 5, and you want to perform But I agree it is confusing. This is a perfect example why I need a fresh pair of eyes 😃 Should I write up a better docstring or would you like to have a go at it? |
So looking at the source code: def haroldpolyadd(*args, trim_zeros=True):
"""
Similar to official polyadd from numpy but allows for
multiple args and doesn't invert the order,
"""
if trim_zeros:
trimmedargs = [np.trim_zeros(x, 'f') for x in args]
else:
trimmedargs = args
degs = [len(m) for m in trimmedargs] # Get the max len of args
s = np.zeros((1, max(degs)))
for ind, x in enumerate(trimmedargs):
s[0, max(degs)-degs[ind]:] += np.real(x)
return s[0] The if-statement should be used for the if trim_zeros:
return np.trim_zeros(s[0], 'f')
else:
return s[0] I'd change this and then improve the docstring with some more information if that's alright? 😄 Maybe even a link to the numpy docs would be nice. |
I think I didn't understand this part but go for it anyways. We can discuss it more concretely over a PR
This is always fine 😃 Please add new tests if you add/remove functionalities under the tests folder
Very good suggestion! I will try to setup a mechanism for inter-library referrals (I think I know how to do it). |
Current docstring contents are not sufficient. Every function, regardless of trivialness, require at least one usage example.
The text was updated successfully, but these errors were encountered: