Rewrite docstrings to match scikit-learn style and show type hints correctly.

[desilinguist]

We should try and match the way that scikit-learn writes their docstrings. Here is an example.

For example, there is a blank line between the parameters and the default values are indicated right after the parameter type. It's also nice how the types that are numpy arrays also include the shapes.

[RobertImbrie]

Working on this now. Since this affects a huge number of files, it's also going to be a huge pull request. Would you prefer me to break it up in some way? If so, how?

[desilinguist]

Good question! How about one module at a time? @mulhod @aoifecahill thoughts?

[mulhod]

Yeah, one module at a time sounds good!

[desilinguist]

@RobertImbrie I am going to add you as an external collaborator on this project so that you can submit pull requests directly in this repo rather than via your fork. This will make sure that Gitlab CI will run properly since you are about to embark on larger PRs.

Thanks again!

[RobertImbrie]

@desilinguist Perfect! Just to clarify, I should:

1. Create a new branch in the EducationalTestingService/skll repo, called Rewrite docstrings or something similar.
2. Whenever I finish changing a module, push to Rewrite docstrings and make a pull request

Is this correct?

(Sorry if this is an obvious question! I'm still new to this and wary of stepping on toes.)

[desilinguist]

Thank you for asking! I think it might make more sense to make a new branch for every module since the branches get auto-deleted after being merged. So something like rewrite-docstrings-<modulename> and then make a new PR each time? Hope that makes sense?

[RobertImbrie]

Yes, it does! I'll make sure to do that. Thanks for the help!

[desilinguist]

Sure! Thank you!

[desilinguist]

Hi @RobertImbrie ! Hope you are well! Are you still available to help us out with this?

[RobertImbrie]

Hi Nitin!

Apologies for the slow response! I found a new job beginning in May, so I've been pretty swamped. I have a project I'm finishing up right now, but I should have some time to tackle this within the next few weeks.

On Thu, Sep 23, 2021, 1:04 PM Nitin Madnani @.***> wrote:

Hi @RobertImbrie https://github.com/RobertImbrie ! Hope you are well! Are you still available to help us out with this?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/EducationalTestingService/skll/issues/647#issuecomment-926037675, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADS4PWFX3RJCI5KNEHNJTEDUDNT4RANCNFSM4UC6BS4Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[desilinguist]

No worries at all! Thank you for getting back to me and it will be great to have contributions from you again when you are ready :)

[RobertImbrie]

Hey @desilinguist! Finally have some time to get started on this again.

I noticed there's inconsistencies between how we write boolean docstrings and how scikit writes them. Even worse, scikit itself is inconsistent with how it writes boolean docstrings!

I want to make our documentation for booleans consistent. That leaves with us three options:

skll's current method: Should we do X or not? scikit method 1: If True, do X. scikit method 2: Whether or not to do X.

I'm leaning towards scikit method 1, though I also don't want to make any unilateral changes. What are your thoughts?

[desilinguist]

Welcome back @RobertImbrie! We really appreciate your contributions.

That's a very interesting finding regarding Boolean docstrings! I think I also agree with your thinking and like scikit option 1 - the most important thing is consistency.

What do you think @mulhod @Frost45 @aoifecahill?

[mulhod]

Yeah, sounds good. I agree about merely being consistent.