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

New expression blocks #479

Merged
merged 4 commits into from
Feb 7, 2022
Merged

New expression blocks #479

merged 4 commits into from
Feb 7, 2022

Conversation

vincentsarago
Copy link
Member

@vincentsarago vincentsarago commented Feb 4, 2022

closes #477

proposed by @geospatial-jeff in slack: use ; instead of , to support multi-blocks expression.

To avoid breaking change/major release, we will still support , delimited blocks, so users who wants to use , in expression HAVE TO put a ; at the end of the expression

expression="where((b1==1) | (b1 > 0.5),1,0)" -> ["where((b1==1) | (b1 > 0.5)" , "1" ,"0)"]  # Not OK

expression="where((b1==1) | (b1 > 0.5),1,0);" -> ["where((b1==1) | (b1 > 0.5),1,0)"]  # OK

I understand this is not optimal but IMO it's the better solution (to my knowledge) to avoid using complex regex or hacky solution. But I'm 👂

Note:
https://numexpr.readthedocs.io/projects/NumExpr3/en/latest/user_guide.html#supported-functions
where(bool, number1, number2), arctan2(float1, float2), {real,imag}(complex), complex(float, float), sum(number, axis=None) and prod(number, axis=None) are the NumExpr function which could need a ,.

@iferencik
Copy link

@vincentsarago thank you for your super quick fix. I plan to use these features to experiment with doing some raster algebra
on the fly. After this merge do I need to update rio-tiler from github?

@vincentsarago
Copy link
Member Author

I'll try to have a release today if @geospatial-jeff and @kylebarron help me 😄

as mentioned in ☝️ to be able to use complex expression you'll need to add ; at the end of the expression.

Copy link
Member

@geospatial-jeff geospatial-jeff left a comment

Choose a reason for hiding this comment

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

I can't think of a better way to do this that isn't breaking. A little uncomfortable that we aren't fixing the underlying problem (relying on character in numexpr to split expressions), but I understand the desire to not cut a new major version just for this and I think the deprecation warning should be enough for now.

This should work until 4.0 comes around!

CHANGES.md Outdated Show resolved Hide resolved
rio_tiler/expression.py Outdated Show resolved Hide resolved
vincentsarago and others added 2 commits February 7, 2022 08:53
Co-authored-by: Kyle Barron <[email protected]>
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.

expression issues
4 participants