Skip to content

Use PyInt::try_to_primitive. - #3187

Merged
youknowone merged 1 commit into
RustPython:mainfrom
DimitrisJim:use_pyint_try_to_prim
Oct 2, 2021
Merged

Use PyInt::try_to_primitive.#3187
youknowone merged 1 commit into
RustPython:mainfrom
DimitrisJim:use_pyint_try_to_prim

Conversation

@DimitrisJim

@DimitrisJim DimitrisJim commented Oct 2, 2021

Copy link
Copy Markdown
Member

clean up after #3142

@DimitrisJim
DimitrisJim force-pushed the use_pyint_try_to_prim branch from b38f143 to 0f1e427 Compare October 2, 2021 01:44
@DimitrisJim
DimitrisJim requested a review from youknowone October 2, 2021 01:45
youknowone
youknowone reviewed Oct 2, 2021
View reviewed changes
Comment thread vm/src/iterator.rs Outdated
.try_to_primitive(vm)
{
Ok(hint) => Ok(Some(hint)),
Err(_) => Err(vm.new_value_error("__length_hint__() should return >= 0".to_owned())),

@youknowone youknowone Oct 2, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The original intend looks like distinguising overflow error and negative value error. Is this still covered well?

@DimitrisJim DimitrisJim Oct 2, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ah, good point. I'll address that.

@youknowone

youknowone commented Oct 2, 2021

Copy link
Copy Markdown
Member

An idea. Sometimes, there will be some features we cannot move from BigInt to PyInt. Would it be better if I didn't move try_to_primitive into PyInt but adding trait PyIntExt and impl it for BigInt?

@DimitrisJim
DimitrisJim force-pushed the use_pyint_try_to_prim branch from 0f1e427 to 2f6deb8 Compare October 2, 2021 16:19
@DimitrisJim

DimitrisJim commented Oct 2, 2021

Copy link
Copy Markdown
Member Author

@youknowone are there places that would need that now? try_from_primitive was the last (as far as I can see) in int.rs that was a free-standing public function that accepted a BigInt.

@youknowone

youknowone commented Oct 2, 2021

Copy link
Copy Markdown
Member

Probably try_f64_to_bigint and try_bigint_to_f64 are one of those example for same relationship. And then I also think try_from_primitive is also one of the member of the group

@youknowone
youknowone merged commit e17d20c into RustPython:main Oct 2, 2021
@DimitrisJim
DimitrisJim deleted the use_pyint_try_to_prim branch November 7, 2021 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@DimitrisJim @youknowone