Skip to content

added test of else method using differently ordered columns #682

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.

Sign up for GitHub

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

Open
epinzur wants to merge 1 commit into
base: main
Choose a base branch
from
Open

added test of else method using differently ordered columns #682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions python/pytests/else_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,3 +65,36 @@ async def test_else_debug(record_source, golden) -> None:
}
)
)


@pytest.fixture(scope="module")
def record_source_slack() -> kd.sources.JsonlString:
content = "\n".join(
[
"""{"text":"Thread 1","user":"UCZ4","time":1,"thread_ts":1,"key":"dev"}""",
Copy link
Collaborator

@bjchambers bjchambers Aug 22, 2023

Choose a reason for hiding this comment

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

Shouldn't use """...""" here -- those are "docstrings". Lint will likely suggest changing it.

"""{"text":"Thread 2","user":"U016","time":2,"thread_ts":1,"key":"dev"}""",
"""{"text":"Msg 1","user":"U016","time":3,"thread_ts":null,"key":"dev"}""",
"""{"text":"Msg 2","user":"U016","time":4,"thread_ts":null,"key":"dev"}""",
]
)
return kd.sources.JsonlString(
Copy link
Collaborator

@bjchambers bjchambers Aug 22, 2023

Choose a reason for hiding this comment

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

PyList may be even easier for this example:

source = kd.sources.PyList(
        [
            {"time": "1996-12-19T16:39:57", "text": "Thread 2", "user": "U016", "thread_ts": 1, "key": "dev" },
            ...,
        ],
        time_column_name="time",
        key_column_name="user",
    )

content, time_column_name="time", key_column_name="key"
)


def test_else_unordered_record(record_source_slack, golden) -> None:
threads = record_source_slack.filter(record_source_slack.col("thread_ts").is_not_null())
non_threads = record_source_slack.filter(record_source_slack.col("thread_ts").is_null())

# this call re-orders the columns in the non_threads timestream
Copy link
Collaborator

@bjchambers bjchambers Aug 22, 2023

Choose a reason for hiding this comment

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

Looking at the error, it doesn't seem to care that the fields are re-ordered. Instead, it doesn't like row 3:

  At positional index 2, first diff: {'user': None, 'text': None, 'time': None, 'thread_ts': None, 'key': None} != {'text': 'Msg 1', 'user': 'U016', 'time': 3, 'thread_ts': None, 'key': 'dev'}

# and causes the bug to occur
non_threads = non_threads.extend({"user": non_threads.col("user")})

joined = kd.record({"threads": threads, "non_threads": non_threads})

threads = joined.col("threads")
non_threads = joined.col("non_threads")

golden.jsonl(
joined.extend({"joined": threads.else_(non_threads)})
Copy link
Collaborator

@bjchambers bjchambers Aug 22, 2023

Choose a reason for hiding this comment

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

Ah. I believe the problem here is that we're creating a new record containing joined. Even when threads.else_(non_threads) is null, this record will sometimes be not null. Eg.

Two questions / thoughts:

  1. Why do we need the joined at all? What happens if you just output threads.else_(non_threads)
  2. If we could do joined.extend(threads.else_(non_threads) that would also help (and it is currently planned)

)
4 changes: 4 additions & 0 deletions python/pytests/golden/else_test/test_else_unordered_record.jsonl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{"_time":"1970-01-01T00:00:00.000000001","_subsort":0,"_key_hash":17095134351192101601,"_key":"dev","joined":{"text":"Thread 1","user":"UCZ4","time":1,"thread_ts":1.0,"key":"dev"},"threads":{"text":"Thread 1","user":"UCZ4","time":1.0,"thread_ts":1.0,"key":"dev"},"non_threads":null}
Copy link
Contributor

@kevinjnguyen kevinjnguyen Aug 21, 2023

Choose a reason for hiding this comment

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

Can you elaborate on what is the expected results here? These appear to be correct. If threads is null then non_threads is used as the value of joined.

Copy link
Collaborator Author

@epinzur epinzur Aug 21, 2023

Choose a reason for hiding this comment

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

the expected results are correct. the test currently fails. https://github.com/kaskada-ai/kaskada/actions/runs/5926509057/job/16068060314?pr=682#step:6:211

{"_time":"1970-01-01T00:00:00.000000002","_subsort":1,"_key_hash":17095134351192101601,"_key":"dev","joined":{"text":"Thread 2","user":"U016","time":2,"thread_ts":1.0,"key":"dev"},"threads":{"text":"Thread 2","user":"U016","time":2.0,"thread_ts":1.0,"key":"dev"},"non_threads":null}
{"_time":"1970-01-01T00:00:00.000000003","_subsort":2,"_key_hash":17095134351192101601,"_key":"dev","joined":{"text":"Msg 1","user":"U016","time":3,"thread_ts":null,"key":"dev"},"threads":null,"non_threads":{"text":"Msg 1","user":"U016","time":3.0,"thread_ts":null,"key":"dev"}}
{"_time":"1970-01-01T00:00:00.000000004","_subsort":3,"_key_hash":17095134351192101601,"_key":"dev","joined":{"text":"Msg 2","user":"U016","time":4,"thread_ts":null,"key":"dev"},"threads":null,"non_threads":{"text":"Msg 2","user":"U016","time":4.0,"thread_ts":null,"key":"dev"}}