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

Support February 29 month end date for leap year #20

Open
qwertytam opened this issue Sep 4, 2021 · 5 comments
Open

Support February 29 month end date for leap year #20

qwertytam opened this issue Sep 4, 2021 · 5 comments

Comments

@qwertytam
Copy link

Some companies use February for their end month for the fiscal year e.g. CarMax and their 10-K with last day as Feb 29, 2016 https://www.sec.gov/Archives/edgar/data/0001170010/000117001016000117/kmx0229201610-k.htm

Currently _check_day use 2001 a non-leap year to validate the day of the month:

def _check_day(month, day):
    """Check if day is a valid day of month.
    :param month: The month to test
    :param day: The day to test
    :return: The day
    :rtype: int
    :raises TypeError: If month or day is not an int or int-like string
    :raises ValueError: If month or day is out of range
    """
    month = _check_month(month)
    day = _check_int(day)

    # Find the last day of the month
    # Use a non-leap year
    max_day = calendar.monthrange(2001, month)[1]

Resolution would be to use year if provided e.g.

def _check_day(day, month, year=None):
    """Check if day is a valid day of month.
    :param day: The day to test
    :param month: The month to test
    :param day: Optional; the year to test; defaults to 2001 a non-leap year
    :return: The day
    :rtype: int
    :raises TypeError: If month or day is not an int or int-like string
    :raises ValueError: If month or day is out of range
    """
    year = year if year else 2001
    month = _check_month(month)
    day = _check_int(day)

    # Find the last day of the month
    max_day = calendar.monthrange(year, month)[1]

Note, in this example, changed order of args (for OCD purposes :) with year last as optional

@adamjstewart
Copy link
Owner

Interesting, I didn't realize that someone might want to use a leap day/month in this way.

I wonder if we should just change 2001 to be a leap year. That would keep the code simple but also allow for this behavior. Thoughts?

@adamjstewart
Copy link
Owner

Also, the logic you pointed out is only used to define the start of a fiscal calendar, not the end. Isn't is already possible to have a fiscal calendar that ends on Fed 29th?

@nicmendoza
Copy link
Contributor

nicmendoza commented Sep 7, 2021

If I understand the thread correctly, yes it does seem you can already have a fiscal year that ends on Feb 29th.

>>> import fiscalyear
>>> fiscalyear.setup_fiscal_calendar(start_month=3)
>>> year = fiscalyear.FiscalYear(2016)
>>> year.end
FiscalDateTime(2016, 2, 29, 23, 59, 59)

Additional thought: We could consider adding an explicit error case for anyone attempting to call it with those parameters (i.e. _check_day(2, 29)), just for the sake of clarity/completeness. Setting Feb 29th as the start date for a fiscal year seems meaningless (unless we find evidence that there are businesses which use Feb 29th as the start date in leap years and Feb 28th/March 1st in other years?). It might make sense to clarify in the comments that the _check_day function checks whether the day/month arguments are a valid start date for a fiscal year rather than "a valid day of month." And/or perhaps give it a more meaningful name.

@qwertytam
Copy link
Author

Where I initially came across this is the data I'm working with gives fiscal year end not start. From looking through the 10-k/q's etc filed with the SEC, all of them (that I've looked through) give the date for the period ending. Perhaps this issue is better approached by being able to initialize a fiscal calendar with a period ending year/month/day? Just a thought given how the SEC financial reports are organized/reported.

@adamjstewart
Copy link
Owner

Yes, we could definitely modify setup_fiscal_calendar to accept either start_* or end_* (but not both). If you want to submit a PR to make this change I'm happy to review. I'm currently swamped with a paper deadline at the end of the month, so I may not be able to get to this anytime soon.

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

3 participants