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

Create season object and related accessor #54

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

AmineBarkaoui
Copy link

Addresses #53.

Copy link
Member

@paololucchino paololucchino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @AmineBarkaoui! Have a look at the comments below and we can discuss a revised approach next time we speak.

@@ -134,6 +136,26 @@ class DekadPeriod(Period):
_period_cls = Dekad


@xarray.register_dataset_accessor("season")
@xarray.register_dataarray_accessor("season")
class SeasonPeriod(Period):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we will be able to inherit from Period as those objects map time onto a global and mutually exclusive set of segments, while Seasons don't. (ie 2024/10/7 will always be 202410d1, but it will be in different seasons depending on what is the relevant season range).

In practical terms, all the properties in the Period class won't work for seasons, as they rely on a _period_cls that can be instantiated with out additional arguments; but for Season we need to pass the season_range.

We can inherit from AccessorTimeBase though.

from datetime import date, datetime
from typing import List, Optional, Tuple, Union

import pandas as pd
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pandas is not a dependency in hdc-algo


def test_season():
# Test initialization and basic attributes
season = Season([(1, 10), (11, 20), (21, 30)])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's discuss the implementation approach. Here a Season instance is an object that can map dates to specific seasons in time. Compare this to the Dekad class, where each instance is actually a specific Dekad in time. I think we want to replicate the latter approach.

Handles season-based indexing and labeling for time series data using dekads.
"""

def __init__(self, season_ranges: List[Tuple[int, int]]):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably what we want (to mirror the approach in Dekad) is:

class Season:

    def __init__(self, date: Union[str, int, datetime, date, Dekad], season_range: List[Tuple[int, int]]):
        # Some code to convert in flexible date input into an integer based representation of the season
        # For example 201501 could be 2015 season 1.
        self._seas = seas
        # Also store the season_range to allow operations and property calculations
        self.season_range = season_range

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

Successfully merging this pull request may close these issues.

2 participants