Skip to content

Docstring and method behaviour differ #20560

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

Closed
tv3141 opened this issue Mar 30, 2018 · 1 comment · Fixed by #33190
Closed

Docstring and method behaviour differ #20560

tv3141 opened this issue Mar 30, 2018 · 1 comment · Fixed by #33190
Labels

Comments

@tv3141
Copy link
Contributor

tv3141 commented Mar 30, 2018

    def _validate_n(self, n):
        """
        Require that `n` be a nonzero integer.
        Parameters
        ----------
        n : int
        Returns
        -------
        nint : int
        Raises
        ------
        TypeError if `int(n)` raises
        ValueError if n != int(n)
        """
        try:
            nint = int(n)
        except (ValueError, TypeError):
            raise TypeError('`n` argument must be an integer, '
                            'got {ntype}'.format(ntype=type(n)))
        if n != nint:
            raise ValueError('`n` argument must be an integer, '
                             'got {n}'.format(n=n))
        return nint

_validate_n does not test whether n is nonzero.

def _validate_n(self, n):

_validate_n is only used in https://github.com/pandas-dev/pandas/blob/601b8c9c45b3cb06ee4ceaf34456bbfd3f5e5d1d/pandas/tseries/offsets.py

It may be that n does not have to be nonzero. Issue #20517 investigates whether n actually has to be nonzero in a case where n is tested again to be nonzero after calling _validate_n:

def __init__(self, n=1, normalize=False, week=0, weekday=0):
self.n = self._validate_n(n)
self.normalize = normalize
self.weekday = weekday
self.week = week
if self.n == 0:
raise ValueError('N cannot be 0')

@WillAyd
Copy link
Member

WillAyd commented Mar 30, 2018

I assume you aren't facing any issue from an end user perspective but are questioning whether or not this internal method serves a purpose right? If so, do you have a suggestion on what you think it should be doing?

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

Successfully merging a pull request may close this issue.

3 participants