BUG: Allow no . at end if indented

[larsoner]

This should be okay:

        kind : str
            Kind of matplotlib plot, e.g.::

                'foo'

        color : str, default 'blue'

This PR makes it okay by allowing the description not to end with . if the last line is indented.

WIP because I'm going to finish going through the PR06, PR08, PR09, RT04, RT05 errors in MNE-Python and there might be more false alarms.

[larsoner]

Okay everything works, @datapythonista when you get a chance to look (or know a Pandas person who might want to look) this is ready for review/merge from my end

[datapythonista]

Happy with the idea, but I think the implementation would be a bit cleaner if we simply don't join the description in doc_parameters, and we join it when it's used, except for the cases you implemented that you just care about the last element in the list.

The joining and unjoining, and the new function feels a bit hacky to me.

Also, do you think it makes sense to not just check on the line starting by space, but also by * and -, so we don't require the period if the line is a list? We'll have to implement this eventually, and I think your PR almost solves it. I think the condition will be quite complex, and worth having a helper function is_last_line_regular that implements it.

[larsoner]

The joining and unjoining, and the new function feels a bit hacky to me.

Agreed, wasn't sure if you had plans for doc_parameters so didn't want to mess with it. But since it's okay to avoid the join there I'll simplify.

Also, do you think it makes sense to not just check on the line starting by space, but also by * and -

Sure, I can add those characters

[larsoner]

@datapythonista pushed a commit with simplifications/cleanups/- support

[datapythonista]

lgtm, nice work

[larsoner]

@datapythonista one more -- I took your idea of refactoring a bit further to refactor both Parameters and Returns checks, since they were redundant. This led to fixing another bug that is presumably in master, which is that directives are not stripped during the Returns desc checks. Now they are (with test added).

[datapythonista]

Good idea. But my preference would be to keep parameter_desc as it was, and just call it from _check_desc. I guess we're not testing parameter_desc, but if we do in the future, will be much easier if every concept is independent. Besides that it may be reused somewhere else in the future.

And to me it'd make things easier to read and understand the code if _check_desc returns errors that are appended to the errors list (instead of receiving errs and modifying it).

And for kinds, instead of the assert I'd simply name them code_no_desc, code_no_upper, code_no_period = kinds. Will make things easier to read.

[larsoner]

But my preference would be to keep parameter_desc as it was, and just call it from _check_desc. I guess we're not testing parameter_desc, but if we do in the future, will be much easier if every concept is independent. Besides that it may be reused somewhere else in the future.

All parameter_desc does is sanitize a the parameter description of the directives. This should be done for both Parameters and Returns. So to keep DRY it should be implemented in a function. If we keep parameter_desc then we need to add returns_desc and sanitize both with a private function (to keep DRY). Would you prefer this to what is there now?

[datapythonista]

Checked it again, and happy with what you did. I didn't check in detail parameter_desc and misunderstood what it was doing. May be having a function remove_directives with most of its code could make sense, but your changes look all right, and I think it's actually clearer that part.

[larsoner]

May be having a function remove_directives with most of its code could make sense

Yeah we might want something like this in the future. But for now since you're happy I'll get this in so we can keep moving (without conflicts) as Pandas and hopefully sklearn give it a shot.