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

[MapD] Added crossjoin operator #1655

Closed
wants to merge 8 commits into from

Conversation

xmnlab
Copy link
Contributor

@xmnlab xmnlab commented Oct 12, 2018

This PR fixes #1621

@cpcloud cpcloud added bug Incorrect behavior inside of ibis expressions Issues or PRs related to the expression API mapd labels Oct 12, 2018
@cpcloud cpcloud added this to the Next Release milestone Oct 12, 2018
@@ -150,6 +151,10 @@ def get_result(self):
fmt_preds = util.indent('USING ' + ', '.join(fmt_preds),
self.indent * 2)
buf.write(fmt_preds)
else:
buf.write(
util.indent('ON (TRUE)', self.indent * 2)
Copy link
Member

Choose a reason for hiding this comment

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

Does this need the parentheses?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it is not necessary ... thanks!

@@ -324,6 +324,13 @@ def _set_literal_format(translator, expr):
return '({})'.format(', '.join(formatted))


def _cross_join(translator, expr):
args = expr.op().args
t1, t2 = args[:2]
Copy link
Member

Choose a reason for hiding this comment

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

I think these have names, like left and right. Can you use those if they exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right! the names are left and right. thanks!

def _cross_join(translator, expr):
args = expr.op().args
t1, t2 = args[:2]

Copy link
Member

Choose a reason for hiding this comment

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

Kill this newline.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

@xmnlab
Copy link
Contributor Author

xmnlab commented Oct 15, 2018

@cpcloud the conda build is failing for windows. do you have any idea about this error?

@cpcloud cpcloud added feature Features or general enhancements and removed bug Incorrect behavior inside of ibis labels Oct 20, 2018
@codecov
Copy link

codecov bot commented Oct 23, 2018

Codecov Report

Merging #1655 into master will decrease coverage by 2.61%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1655      +/-   ##
==========================================
- Coverage   89.94%   87.33%   -2.62%     
==========================================
  Files         185      185              
  Lines       27209    27219      +10     
  Branches     2328     2337       +9     
==========================================
- Hits        24474    23771     -703     
- Misses       2335     3040     +705     
- Partials      400      408       +8
Impacted Files Coverage Δ
ibis/mapd/compiler.py 79.67% <100%> (+9.18%) ⬆️
ibis/mapd/tests/test_operations.py 95.45% <100%> (+0.21%) ⬆️
ibis/mapd/operations.py 71% <25%> (-0.1%) ⬇️
ibis/bigquery/tests/test_client.py 25.87% <0%> (-73.55%) ⬇️
ibis/bigquery/tests/test_compiler.py 39.89% <0%> (-60.11%) ⬇️
ibis/bigquery/udf/tests/test_udf_execute.py 28% <0%> (-54.18%) ⬇️
ibis/bigquery/client.py 42.91% <0%> (-52.37%) ⬇️
ibis/bigquery/compiler.py 58.82% <0%> (-38.61%) ⬇️
ibis/bigquery/tests/conftest.py 70.27% <0%> (-16.22%) ⬇️
ibis/bigquery/udf/api.py 80.48% <0%> (-14.64%) ⬇️
... and 31 more

@xmnlab
Copy link
Contributor Author

xmnlab commented Oct 25, 2018

same problem with mariadb on windows test py35

@cpcloud cpcloud closed this in 0627924 Oct 28, 2018
@cpcloud
Copy link
Member

cpcloud commented Oct 28, 2018

Thanks @xmnlab !

@xmnlab xmnlab deleted the add_crossjoin branch March 27, 2019 01:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
expressions Issues or PRs related to the expression API feature Features or general enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MapD histogram() : KeyError: <class 'ibis.expr.operations.CrossJoin'>
2 participants