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

feat(types): isoday_of_week method #9003

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

Conversation

kaijennissen
Copy link
Contributor

Description of changes

Added a new isoday_of_week method for timestamps.

Issues closed

Resolves #8989

Copy link
Contributor

ACTION NEEDED

Ibis follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message.

Please update your PR title and description to match the specification.

@kaijennissen kaijennissen changed the title isoday_of_week method feat(types): isoday_of_week method Apr 18, 2024
@cpcloud
Copy link
Member

cpcloud commented Apr 18, 2024

We have a DayOfWeek class defined in ibis/expr/types/temporal.py.

It's a bit of an oddball API in that it functions like a namespace instead of top level method like everything else.

Naturally, your first development experience with Ibis is extending a slightly weird bit of the codebase 😂.

Perhaps we can add two new methods there?

class DayOfWeek:
    ...

    def iso_index(self): ...
    def iso_full_name(self): ...

for consistency with the surrounding methods?

@cpcloud
Copy link
Member

cpcloud commented Apr 18, 2024

Here's a usage example:

In [8]: t = ibis.read_parquet("/data/avg_duration_over_time.parquet")

In [9]: t.head()
Out[9]:
┏━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━┓
┃ date       ┃ avg_duration_m ┃
┡━━━━━━━━━━━━╇━━━━━━━━━━━━━━━━┩
│ date       │ float64        │
├────────────┼────────────────┤
│ 2024-04-16 │      57.833333 │
│ 2024-04-15 │      42.833333 │
│ 2024-04-14 │      31.500000 │
│ 2024-04-13 │      53.600000 │
│ 2024-04-12 │      47.400000 │
└────────────┴────────────────┘

In [10]: t.select(
    ...:     "date",
    ...:     weekday=t.date.day_of_week.index(),
    ...:     weekday_name=t.date.day_of_week.full_name(),
    ...: )
Out[10]:
┏━━━━━━━━━━━━┳━━━━━━━━━┳━━━━━━━━━━━━━━┓
┃ date       ┃ weekday ┃ weekday_name ┃
┡━━━━━━━━━━━━╇━━━━━━━━━╇━━━━━━━━━━━━━━┩
│ date       │ int16   │ string       │
├────────────┼─────────┼──────────────┤
│ 2024-04-16 │       1 │ Tuesday      │
│ 2024-04-15 │       0 │ Monday       │
│ 2024-04-14 │       6 │ Sunday       │
│ 2024-04-13 │       5 │ Saturday     │
│ 2024-04-12 │       4 │ Friday       │
│ 2024-04-10 │       2 │ Wednesday    │
│ 2024-04-09 │       1 │ Tuesday      │
│ 2024-04-07 │       6 │ Sunday       │
│ 2024-04-06 │       5 │ Saturday     │
│ 2024-04-05 │       4 │ Friday       │
│ …          │       … │ …            │
└────────────┴─────────┴──────────────┘

@cpcloud
Copy link
Member

cpcloud commented Apr 18, 2024

Alternatively, I can take a crack at this one, if you want to handle iso_year()

@kaijennissen
Copy link
Contributor Author

kaijennissen commented Apr 18, 2024

We have a DayOfWeek class defined in ibis/expr/types/temporal.py.

I missed that.

Alternatively, I can take a crack at this one, if you want to handle iso_year()

I'll give it a try. And iso_yearwith a different PR.

@kaijennissen
Copy link
Contributor Author

Is it possible that there are currently no tests for .day_of_week.index and .day_of_week.full_name? Was looking to place a test nearby but couldn't find them.

@cpcloud
Copy link
Member

cpcloud commented Apr 19, 2024

@kaijennissen You should be able to find the tests that run across backends in ibis/backends/tests/test_temporal.py:

❯ rg test_day_of_week
ibis/backends/bigquery/tests/unit/test_compiler.py
72:def test_day_of_week(case, dtype, snapshot):

ibis/backends/impala/tests/test_client.py
202:def test_day_of_week(con):

ibis/backends/tests/test_temporal.py
1463:def test_day_of_week_scalar(con, date, expected_index, expected_day):
1484:def test_day_of_week_column(backend, alltypes, df):
1529:def test_day_of_week_column_group_by(

@kaijennissen
Copy link
Contributor Author

kaijennissen commented Apr 20, 2024

@cpcloud The sqlite tests pass locally. The exasol as well as the impala containers do not work thus I cannot test this locally. I have to investigate this a little deeper. I have no idea why the docs-test fails.

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.

Possibility to extract isoyear and isoweekday (or weekday) from a timestamp column.
2 participants