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

Dataframe v2: big ol' hindsight-driven refactoring #7683

Merged
merged 3 commits into from
Oct 11, 2024

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Oct 10, 2024

Simpler. Faster. Correct-er.

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!
  • If have noted any breaking changes to the log API in CHANGELOG.md and the migration guide

To run all checks from main, comment on the PR with @rerun-bot full-check.

@teh-cmc teh-cmc added 🪳 bug Something isn't working 🚀 performance Optimization, memory use, etc 🚜 refactor Change the code, not the functionality exclude from changelog PRs with this won't show up in CHANGELOG.md feat-dataframe-api Everything related to the dataframe API labels Oct 10, 2024
@teh-cmc teh-cmc changed the base branch from main to cmc/store_empty_writes October 10, 2024 19:47
@teh-cmc teh-cmc added the do-not-merge Do not merge this PR label Oct 10, 2024
Comment on lines +1560 to +1555
Timestamp(Nanosecond, None)[None, None, 1970-01-01 00:00:00.000000050, 1970-01-01 00:00:00.000000070],
ListArray[[2], [3], [4], [6]],
ListArray[None, None, None, None],
ListArray[None, None, None, None],
ListArray[[{x: 2, y: 2}], [{x: 3, y: 3}], [{x: 4, y: 4}], [{x: 8, y: 8}]],
Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, the expected value was wrong 😒

@teh-cmc teh-cmc marked this pull request as ready for review October 11, 2024 07:26
Base automatically changed from cmc/store_empty_writes to main October 11, 2024 12:59
@teh-cmc teh-cmc force-pushed the cmc/dataframev2_hindsight branch from 8e5a8d7 to 1ebc0ba Compare October 11, 2024 13:00
Copy link
Member

@jleibs jleibs left a comment

Choose a reason for hiding this comment

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

This is so much easier to follow!

@teh-cmc teh-cmc removed the do-not-merge Do not merge this PR label Oct 11, 2024
@teh-cmc teh-cmc merged commit 9f23ae0 into main Oct 11, 2024
29 of 30 checks passed
@teh-cmc teh-cmc deleted the cmc/dataframev2_hindsight branch October 11, 2024 13:07
jleibs added a commit that referenced this pull request Oct 11, 2024
…t`, and `filter_is_not_null` (#7680)

### What
- Resolves: #7455
- DNM: requires #7683 

### TODO
- [x] The unit test currently fails with what looks like a bug

### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested the web demo (if applicable):
* Using examples from latest `main` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7680?manifest_url=https://app.rerun.io/version/main/examples_manifest.json)
* Using full set of examples from `nightly` build:
[rerun.io/viewer](https://rerun.io/viewer/pr/7680?manifest_url=https://app.rerun.io/version/nightly/examples_manifest.json)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG
* [x] If applicable, add a new check to the [release
checklist](https://github.com/rerun-io/rerun/blob/main/tests/python/release_checklist)!
* [x] If have noted any breaking changes to the log API in
`CHANGELOG.md` and the migration guide

- [PR Build Summary](https://build.rerun.io/pr/7680)
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)

To run all checks from `main`, comment on the PR with `@rerun-bot
full-check`.

---------

Co-authored-by: Clement Rey <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🪳 bug Something isn't working exclude from changelog PRs with this won't show up in CHANGELOG.md feat-dataframe-api Everything related to the dataframe API 🚀 performance Optimization, memory use, etc 🚜 refactor Change the code, not the functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

filter_is_not_null / filtered_point_of_view returns incorrect results
2 participants