Skip to content

Wrapping the Rust MatchSpec in Python#16

Merged
remkade merged 4 commits intoremkade:mainfrom
beeankha:python_wrapper
Nov 10, 2022
Merged

Wrapping the Rust MatchSpec in Python#16
remkade merged 4 commits intoremkade:mainfrom
beeankha:python_wrapper

Conversation

@beeankha
Copy link
Collaborator

@beeankha beeankha commented Nov 9, 2022

Addresses this issue:


To run this, enter the following command from the directory where the matchspec_in_rust.py is located:

$ maturin develop

Copy link
Owner

@remkade remkade left a comment

Choose a reason for hiding this comment

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

Not sure how maturin likes to handle errors, but maybe this will work?

Comment on lines 11 to 14
fn match_against_matchspec(matchspec: String, package: String, version: String) -> bool {
let ms: matchspec::MatchSpec<String> = matchspec.parse();
ms.is_package_version_match(&package, &version)
}
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe the correct way to do this is by returning a PyResult? That way we don't lose the possible error. Maybe this?

Suggested change
fn match_against_matchspec(matchspec: String, package: String, version: String) -> bool {
let ms: matchspec::MatchSpec<String> = matchspec.parse();
ms.is_package_version_match(&package, &version)
}
fn match_against_matchspec(matchspec: String, package: String, version: String) -> PyResult<bool> {
let ms: matchspec::MatchSpec<String> = matchspec.parse()?;
Ok(ms.is_package_version_match(&package, &version))
}

That question mark after the parse() will return the error if there is one, otherwise will unwrap the Ok(MatchSpec<String>) and give just the MatchSpec<String>.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried using unwrap() as suggested by @marcoesters (suggestion was made outside of this PR) and it seems to work!

@remkade remkade enabled auto-merge November 10, 2022 20:30
@remkade remkade merged commit 4d3bb3a into remkade:main Nov 10, 2022
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.

3 participants