put frequency_quantity closer to frequency_unit (renamed)

I intentionally added it in the end because I want frequency_quantity to have a default value = 1 to make it backward compatible. With default value, it cannot be placed before any argument without default value. Even if it is the first one among arguments with default value, it is still far from frequency_unit so I put it in the end.

Can we instead pass in Frequency as a dataclass?

@dataclass
class Frequency:
    """Class to specify forecast frequency."""
    frequency_unit: str
    frequency_value: int = 1

ok the problem is that for some functions, I want frequency_quantity has default value = 1. These functions are the ones that are used outside, in the trial notebook. If a customer developed based on an old trial notebook, we do not want to break those function calls in the old trial notebooks.

For other functions that are only called inside this repo, I recommend having no default values so it can expose bugs earlier. Sometimes with default value, some bugs may be silently ignored.

Besides, I believe we are around the corner to refactor this repo and core library code, as we now are pretty sure we will go black-box. So I would suggest deferring introducing any complex data structures for now.

Is this code shared between serverless and classic? If so, then it makes sense. If not, why worried about trial notebook?

Oh I was wrong, you are right. This code is only used by serverless.
Let me clarify the tradeoffs of introducing a new dataclass.
Pros: code is cleaner. (But I doubt how much it improved the code readability. I believe without the dataclass, it is still pretty readable)
Cons:

1. Need to change a lot of code and their related tests, including some codes that are not necessarily touched in this PR.
2. This dataclass will force me to either all have default values or all without default values. But we actually do not need to have default values now. But anyway it makes the code lose flexibility.

I would still prefer not using such dataclass. wdyt?

It's not just cleaner code (in terms of readability) but better for overall maintainability and testability.

Think about the following:

1. The current argument list is already long. For the consumer of this function, they need to read many argument definitions in order to understand how to use this
2. The frequency_unit and frequency_quantity is a combined logic concept. Specify either one of the doesn't make sense.
3. Easy to validate: you can add a validate function inside the dataclass. So every time a Frequency object is passed in, it can be validated immediately.
4. Easy test: you only need to test the Frequency object instead of two separate values. You can have a simple test like def is_valid_frequency(freq: Frequency) -> bool. In the future, if you want to support other quantities, e.g. 45min or 3d, you don't need to add check all over the place.
5. Having a default value in it is not a bad thing. With single object, you can change the default value only once and it applies to all.

Anyway, given the urgency of this task, I don't want to make this a blocker. But I suggest you create a follow-up JIRA in EE epic to refactor this code.

jira ticket, https://databricks.atlassian.net/jira/software/c/projects/ML/boards/3737?selectedIssue=ML-50316

what if it's not divisible, we just get the floor value? is that intended?

yes it is intended. If it is not divisible, we will get the floor value. And in the later check we will use this floor value to find out it is not consistent.

please add comment in here.

Still not clear to me. What is this == check doing here? Please also add parenthesis around to make the compute precedence clearer.

on the left of this ==, it calculates the difference between end time and period time, on the right, it is start time. the variable diff is True or False