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.

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

Jump to bottom

feat(common): also traverse nodes used as dictionary keys #9041

Merged
merged 1 commit into from
Apr 25, 2024
Merged

feat(common): also traverse nodes used as dictionary keys #9041

merged 1 commit into from
Apr 25, 2024

Conversation

kszucs
Copy link
Member

@kszucs kszucs commented Apr 23, 2024 •
edited
Loading

This change allows repr-ing node mappings, like the dereference mappings for easier inspection.

Inevitably this is slowing down the traversals, but changing the instance checks to use a plain dict instead of (dict, frozendict) maintains the previous traversal speed:

---------------------------------------------------------------------------- benchmark 'test_bfs': 2 tests -----------------------------------------------------------------------------
Name (time in ms)              Min               Max              Mean            StdDev            Median               IQR            Outliers       OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_bfs (0738_9355281)     1.3995 (1.0)      1.5527 (1.05)     1.4131 (1.0)      0.0173 (2.86)     1.4101 (1.0)      0.0094 (2.24)        22;22  707.6590 (1.0)         442           1
test_bfs (NOW)              1.4234 (1.02)     1.4826 (1.0)      1.4338 (1.01)     0.0060 (1.0)      1.4333 (1.02)     0.0042 (1.0)         42;16  697.4705 (0.99)        370           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

---------------------------------------------------------------------------- benchmark 'test_dfs': 2 tests -----------------------------------------------------------------------------
Name (time in ms)              Min               Max              Mean            StdDev            Median               IQR            Outliers       OPS            Rounds  Iterations
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_dfs (0738_9355281)     1.4105 (1.0)      1.5869 (1.08)     1.4272 (1.0)      0.0222 (4.31)     1.4227 (1.0)      0.0087 (2.09)        29;32  700.6742 (1.0)         381           1
test_dfs (NOW)              1.4381 (1.02)     1.4726 (1.0)      1.4454 (1.01)     0.0051 (1.0)      1.4446 (1.02)     0.0042 (1.0)         41;23  691.8344 (0.99)        418           1
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

----------------------------------------------------------------------------------- benchmark 'test_equality_caching': 2 tests ----------------------------------------------------------------------------------
Name (time in ns)                             Min                 Max                Mean            StdDev              Median               IQR            Outliers  OPS (Mops/s)            Rounds  Iterations
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_equality_caching (0738_9355281)     151.2501 (1.01)     205.8300 (1.15)     152.8526 (1.00)     4.5554 (2.08)     152.0900 (1.00)     0.4200 (1.0)          4;27        6.5423 (1.00)        200         100
test_equality_caching (NOW)              150.4200 (1.0)      178.7501 (1.0)      152.3245 (1.0)      2.1914 (1.0)      152.0800 (1.0)      0.8300 (1.98)         8;15        6.5649 (1.0)         200         100
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

----------------------------------------------------------------------------- benchmark 'test_replace_mapping': 2 tests -----------------------------------------------------------------------------
Name (time in ms)                          Min                Max              Mean            StdDev            Median               IQR            Outliers       OPS            Rounds  Iterations
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_replace_mapping (0738_9355281)     7.3154 (1.0)      28.8150 (1.05)     7.6806 (1.02)     2.5469 (1.42)     7.3525 (1.0)      0.0231 (1.69)          2;6  130.1986 (0.98)        123           1
test_replace_mapping (NOW)              7.3534 (1.01)     27.3207 (1.0)      7.5403 (1.0)      1.7984 (1.0)      7.3719 (1.00)     0.0137 (1.0)          1;12  132.6202 (1.0)         123           1
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

------------------------------------------------------------------------------ benchmark 'test_replace_pattern': 2 tests ------------------------------------------------------------------------------
Name (time in ms)                           Min                Max               Mean            StdDev             Median               IQR            Outliers      OPS            Rounds  Iterations
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
test_replace_pattern (0738_9355281)     10.5733 (1.0)      30.5281 (1.07)     10.8404 (1.0)      2.1232 (1.10)     10.6082 (1.0)      0.0281 (1.0)           1;4  92.2472 (1.0)          88           1
test_replace_pattern (NOW)              10.6792 (1.01)     28.5771 (1.0)      10.9808 (1.01)     1.9221 (1.0)      10.7539 (1.01)     0.0330 (1.17)          1;8  91.0683 (0.99)         86           1
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

@kszucs
feat(common): also traverse nodes used as dictionary keys
db2563b 
This change allows repr-ing node mappings, like the dereference mappings
for easier inspection.
@kszucs kszucs requested a review from cpcloud April 23, 2024 13:19
@kszucs kszucs mentioned this pull request Apr 23, 2024
Merged
@cpcloud cpcloud added this to the 9.0 milestone Apr 25, 2024
@cpcloud cpcloud added feature Features or general enhancements internals Issues or PRs related to ibis's internal APIs labels Apr 25, 2024
@cpcloud cpcloud merged commit 02c6607 into ibis-project:main Apr 25, 2024
87 checks passed
@cpcloud cpcloud deleted the graph-traverse-dict-keys branch April 25, 2024 13:12
kszucs added a commit that referenced this pull request Apr 25, 2024
@kszucs
feat(ir): more flexible dereferencing support for join right hand side (
d7a31aa 
#8992)

Enables to use fields from parent tables of the join right hand side
instead of enforcing to use the same exact table:

```py
t1 = ibis.table(name="t1", schema={"a": "int64", "b": "string"})
t2 = ibis.table(name="t2", schema={"c": "int64", "d": "string"})

t3 = t2.mutate(e=t2.c + 1)
joined = t1.join(t3, [t1.a == t2.c])  # here we use t2.c instead of t3.c
```

Identify ambiguous cases and raise an error, like the following case:

```py
t.join(t, [t.a == t.a])
```

depends on:
- #9043 
- #9041 

fixes #8581
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cpcloud cpcloud cpcloud approved these changes

Assignees
No one assigned
Labels
feature Features or general enhancements internals Issues or PRs related to ibis's internal APIs
Projects
None yet
Milestone
9.0
Development

Successfully merging this pull request may close these issues.
2 participants
@kszucs @cpcloud