Skip to content
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

GL01 Compliance #19

Open
slackline opened this issue Jan 2, 2024 · 4 comments
Open

GL01 Compliance #19

slackline opened this issue Jan 2, 2024 · 4 comments

Comments

@slackline
Copy link

Thanks for writing and maintaining this package, I find it super useful when writing code.

One thing I've noticed is that the following was automatically generated...

def summarise_shape(shape: npt.NDArray) -> list:
    """Summarise the region properties of a 2D numpy array using Scikit-Image.

    Parameters
    ----------
    shape : npt.NDArray
        2D binary array of a shape.

    Returns
    -------
    list
        List of Region Properties each item describing one labelled region.
    """
    return measure.regionprops(shape)

I use the numpydoc pre-commit hook to make sure I get everything correct and on parsing the above it raised GL01.

Putting the first line on its own new line assuaged the linter.

def summarise_shape(shape: npt.NDArray) -> list:
    """
    Summarise the region properties of a 2D numpy array using Scikit-Image.

    Parameters
    ----------
    shape : npt.NDArray
        2D binary array of a shape.

    Returns
    -------
    list
        List of Region Properties each item describing one labelled region.
    """
    return measure.regionprops(shape)

I'll see if I can work out how to add this and make a Pull Request over the coming weeks but am writing this up now for reference.

@douglasdavis
Copy link
Owner

Thanks for raising the issue! This can definitely be added as a defcustom, something like

(defcustom numpydoc-newline-after-opening-quotes nil
  "Flag to control insertion of newline after the opening quotes.
If set to t a newline will be inserted after the opening quotes of 
the docstring."
  :group 'numpydoc
  :type 'boolean)

(defaulting to nil to maintain existing behavior)

The implementation of numpydoc--insert-short-and-long-desc will then need a small update to use the new custom variable as necessary.

@slackline
Copy link
Author

slackline commented Jan 3, 2024

Thanks for the pointer @douglasdavis, I'll give this a go (first proper foray into Emacs LISP).

I wonder if, longer term, it would be worth following the various validation rules albeit as a global option.

First things first though, I need to walk before I can run.

@douglasdavis
Copy link
Owner

I was actually unaware of GL01! I think that goes to show the value of the validator, because I've seen a lot of projects (including my own) that try to be numpydoc compliant but do not include the newline. So perhaps it would be a good idea to have the default setting be t!

@slackline
Copy link
Author

slackline commented Jan 5, 2024

I've had a first stab at this but am fumbling around to work out how to get the line to indent.

    (numpydoc--insert indent
                      (concat (make-string 3 numpydoc-quote-char)
                              (if numpydoc-newline-after-opening-quotes
                                  "\n"
                                  "")
                              (if (numpydoc--prompt-p)
                                  (read-string
                                   (format "Short description: "))
                                   tmps)
                              "\n\n")
                      (make-string 3 numpydoc-quote-char))

What I don't understand/can't figure out is why, it doesn't indent because the code chunk here is using numpydoc--insert indent LINES where lines is each element if the result of (concat ...).

The concatenated items are...

  1. Three of whatever value numpydoc-quote-char is.
  2. Concatenated with a carriage return conditional on whether numpydoc-newline-after-opening-quotes is non-nil.
  3. The user prompted description or the template from tmps
  4. Two carriage returns.
  5. Three of whatever value numpydoc-quote-char is.

I've tried a variant where I concatenate 1) and 2) first

    (numpydoc--insert indent
                      (concat (concat (make-string 3 numpydoc-quote-char)
                                               (if numpydoc-newline-after-opening-quotes
                                                "\n"
                                                ""))
                              (if (numpydoc--prompt-p)
                                  (read-string
                                   (format "Short description: "))
                                   tmps)
                              "\n\n")
                      (make-string 3 numpydoc-quote-char))

but with the same result.

I was actually unaware of GL01! I think that goes to show the value of the validator, because I've seen a lot of projects (including my own) that try to be numpydoc compliant but do not include the newline. So perhaps it would be a good idea to have the default setting be t!

I think it would make sense and that would just be a case of setting defcustom numpydoc-newline-after-opening-quotes t but still make it configurable if users wanted something different wouldn't it?

There are various other validation rules in there, most are already met e.g. PR10 : Parameter requires a space before the colon separating the parameter name and type. I'd be happy to go through and check the other rules and help get the defaults compliant with them once I've got out the starting blocks with this first rule.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants