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

add validation methods to python bindings #305

Merged

Conversation

Machine-Maker
Copy link
Contributor

Closes #304

@stefan-gorules
Copy link
Contributor

stefan-gorules commented Feb 9, 2025

Hi @Machine-Maker,

Apologies for the late reply. We had to focus on updating Pyo3 to support latest Python, some fixes regarding Python execution and #307.

This looks great! Could you just move the main portion of the code to Rust (maybe to validate.rs) and export these functions.

Then we can call them in Python, and export them to Python as well. We'll handle the case for other languages. Haven't forgotten for scientific JSON notation either, just want to merge couple of higher priority things first, and then we'll merge that one as well.

EDIT: So you don't need to rebase and modify code twice, I'll let you know once #307 is merged, as there are additional changes coming to Python.

@stefan-gorules
Copy link
Contributor

Hi @Machine-Maker,

The pull request linked above has been merged now. Happy to move this forward once above is fixed. Thanks!

@Machine-Maker
Copy link
Contributor Author

Ok, should be done.

Not a super expert in rust, so if this isn't setup the best, please let me know.

@Machine-Maker Machine-Maker force-pushed the feature/python-validate-expression branch from 1747d9e to 8a3c5cd Compare February 13, 2025 19:27
Copy link
Contributor

@stefan-gorules stefan-gorules left a comment

Choose a reason for hiding this comment

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

After these 2 minor changes (naming) happy to merge (note to update also in the caller)

core/expression/src/validate.rs Outdated Show resolved Hide resolved
core/expression/src/validate.rs Outdated Show resolved Hide resolved
@Machine-Maker
Copy link
Contributor Author

Ok, renamed

@stefan-gorules
Copy link
Contributor

Will need you to run cargo fmt --all as well (format pipeline is failing)

@Machine-Maker
Copy link
Contributor Author

Done

@stefan-gorules stefan-gorules merged commit 94e1da2 into gorules:master Feb 15, 2025
43 checks passed
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.

add validation methods to language bindings (python)
2 participants