This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5581

rust-cpython: set conversion for MissingAncestors.bases()
ClosedPublic

Authored by gracinet on Jan 14 2019, 9:14 PM.

Details

Reviewers
kevincox
Group Reviewers
hg-reviewers
Commits
rHG5257e6299d4c: rust-cpython: set conversion for MissingAncestors.bases()
Summary

Also I hope that the separate py_set() helper will help transition
to proper PySet support in rust-cpython

Took the opportunity to replace explict for loop with iteration
and collect().

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

gracinet created this revision.Jan 14 2019, 9:14 PM
kevincox accepted this revision.Jan 15 2019, 2:46 PM
Closed by commit rHG5257e6299d4c: rust-cpython: set conversion for MissingAncestors.bases() (authored by gracinet). · Explain WhyJan 16 2019, 7:44 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 13252

rust/hg-cpython/src/ancestors.rs

// ancestors.rs // ancestors.rs
// //
// Copyright 2018 Georges Racinet <gracinet@anybox.fr> // Copyright 2018 Georges Racinet <gracinet@anybox.fr>
// //
// This software may be used and distributed according to the terms of the // This software may be used and distributed according to the terms of the
// GNU General Public License version 2 or any later version. // GNU General Public License version 2 or any later version.
//! Bindings for the `hg::ancestors` module provided by the //! Bindings for the `hg::ancestors` module provided by the
//! `hg-core` crate. From Python, this will be seen as `rustext.ancestor` //! `hg-core` crate. From Python, this will be seen as `rustext.ancestor`
//! and can be used as replacement for the the pure `ancestor` Python module. //! and can be used as replacement for the the pure `ancestor` Python module.
//! //!
//! # Classes visible from Python: //! # Classes visible from Python:
//! - [`LazyAncestors`] is the Rust implementation of //! - [`LazyAncestors`] is the Rust implementation of
//! `mercurial.ancestor.lazyancestors`. //! `mercurial.ancestor.lazyancestors`. The only difference is that it is
//! The only difference is that it is instantiated with a C `parsers.index` //! instantiated with a C `parsers.index` instance instead of a parents
//! instance instead of a parents function. //! function.
//! //!
//! - [`MissingAncestors`] is the Rust implementation of //! - [`MissingAncestors`] is the Rust implementation of
//! `mercurial.ancestor.incrementalmissingancestors`. //! `mercurial.ancestor.incrementalmissingancestors`.
//! //!
//! API differences: //! API differences:
//! + it is instantiated with a C `parsers.index` //! + it is instantiated with a C `parsers.index`
//! instance instead of a parents function. //! instance instead of a parents function.
//! + `MissingAncestors.bases` is a method returning a tuple instead of //! + `MissingAncestors.bases` is a method returning a tuple instead of
//! a set-valued attribute. We could return a Python set easily if our //! a set-valued attribute. We could return a Python set easily if our
//! [PySet PR](https://github.com/dgrunwald/rust-cpython/pull/165) //! [PySet PR](https://github.com/dgrunwald/rust-cpython/pull/165)
//! is accepted. //! is accepted.
//! //!
//! - [`AncestorsIterator`] is the Rust counterpart of the //! - [`AncestorsIterator`] is the Rust counterpart of the
//! `ancestor._lazyancestorsiter` Python generator. //! `ancestor._lazyancestorsiter` Python generator. From Python, instances of
//! From Python, instances of this should be mainly obtained by calling //! this should be mainly obtained by calling `iter()` on a [`LazyAncestors`]
//! `iter()` on a [`LazyAncestors`] instance. //! instance.
//! //!
//! [`LazyAncestors`]: struct.LazyAncestors.html //! [`LazyAncestors`]: struct.LazyAncestors.html
//! [`MissingAncestors`]: struct.MissingAncestors.html //! [`MissingAncestors`]: struct.MissingAncestors.html
//! [`AncestorsIterator`]: struct.AncestorsIterator.html //! [`AncestorsIterator`]: struct.AncestorsIterator.html
use crate::conversion::rev_pyiter_collect; use crate::conversion::rev_pyiter_collect;
use cindex::Index; use cindex::Index;
use cpython::{ use cpython::{
ObjectProtocol, PyClone, PyDict, PyList, PyModule, PyObject, ObjectProtocol, PyClone, PyDict, PyList, PyModule, PyObject, PyResult,
PyResult, PyTuple, Python, PythonObject, ToPyObject, PyTuple, Python, PythonObject, ToPyObject,
}; };
use exceptions::GraphError; use exceptions::GraphError;
use hg::Revision; use hg::Revision;
use hg::{ use hg::{
AncestorsIterator as CoreIterator, LazyAncestors as CoreLazy, AncestorsIterator as CoreIterator, LazyAncestors as CoreLazy,
MissingAncestors as CoreMissing, MissingAncestors as CoreMissing,
}; };
use std::cell::RefCell; use std::cell::RefCell;
}); });
impl AncestorsIterator { impl AncestorsIterator {
pub fn from_inner(py: Python, ait: CoreIterator<Index>) -> PyResult<Self> { pub fn from_inner(py: Python, ait: CoreIterator<Index>) -> PyResult<Self> {
Self::create_instance(py, RefCell::new(Box::new(ait))) Self::create_instance(py, RefCell::new(Box::new(ait)))
} }
} }
/// Copy and convert an `HashSet<Revision>` in a Python set
///
/// This will probably turn useless once `PySet` support lands in
/// `rust-cpython`.
///
/// This builds a Python tuple, then calls Python's "set()" on it
fn py_set(py: Python, set: &HashSet<Revision>) -> PyResult<PyObject> {
let as_vec: Vec<PyObject> = set
.iter()
.map(|rev| rev.to_py_object(py).into_object())
.collect();
let as_pytuple = PyTuple::new(py, as_vec.as_slice());
let locals = PyDict::new(py);
locals.set_item(py, "obj", as_pytuple.to_py_object(py))?;
py.eval("set(obj)", None, Some(&locals))
}
py_class!(pub class LazyAncestors |py| { py_class!(pub class LazyAncestors |py| {
data inner: RefCell<Box<CoreLazy<Index>>>; data inner: RefCell<Box<CoreLazy<Index>>>;
def __contains__(&self, rev: Revision) -> PyResult<bool> { def __contains__(&self, rev: Revision) -> PyResult<bool> {
self.inner(py) self.inner(py)
.borrow_mut() .borrow_mut()
.contains(rev) .contains(rev)
.map_err(|e| GraphError::pynew(py, e)) .map_err(|e| GraphError::pynew(py, e))
let bases_vec: Vec<Revision> = rev_pyiter_collect(py, &bases)?; let bases_vec: Vec<Revision> = rev_pyiter_collect(py, &bases)?;
inner.add_bases(bases_vec); inner.add_bases(bases_vec);
// cpython doc has examples with PyResult<()> but this gives me // cpython doc has examples with PyResult<()> but this gives me
// the trait `cpython::ToPyObject` is not implemented for `()` // the trait `cpython::ToPyObject` is not implemented for `()`
// so let's return an explicit None // so let's return an explicit None
Ok(py.None()) Ok(py.None())
} }
def bases(&self) -> PyResult<PyTuple> { def bases(&self) -> PyResult<PyObject> {
let inner = self.inner(py).borrow(); py_set(py, self.inner(py).borrow().get_bases())
let bases_set = inner.get_bases();
// convert as Python tuple TODO how to return a proper Python set?
let mut bases_vec: Vec<PyObject> = Vec::with_capacity(
bases_set.len());
for rev in bases_set {
bases_vec.push(rev.to_py_object(py).into_object());
}
Ok(PyTuple::new(py, bases_vec.as_slice()))
} }
def removeancestorsfrom(&self, revs: PyObject) -> PyResult<PyObject> { def removeancestorsfrom(&self, revs: PyObject) -> PyResult<PyObject> {
let mut inner = self.inner(py).borrow_mut(); let mut inner = self.inner(py).borrow_mut();
// this is very lame: we convert to a Rust set, update it in place // this is very lame: we convert to a Rust set, update it in place
// and then convert back to Python, only to have Python remove the // and then convert back to Python, only to have Python remove the
// excess (thankfully, Python is happy with a list or even an iterator) // excess (thankfully, Python is happy with a list or even an iterator)
// Leads to improve this: // Leads to improve this:

tests/test-rust-ancestor.py

self.assertFalse(LazyAncestors(idx, [0], 0, False)) self.assertFalse(LazyAncestors(idx, [0], 0, False))
def testmissingancestors(self): def testmissingancestors(self):
idx = self.parseindex() idx = self.parseindex()
missanc = MissingAncestors(idx, [1]) missanc = MissingAncestors(idx, [1])
self.assertTrue(missanc.hasbases()) self.assertTrue(missanc.hasbases())
self.assertEqual(missanc.missingancestors([3]), [2, 3]) self.assertEqual(missanc.missingancestors([3]), [2, 3])
missanc.addbases({2}) missanc.addbases({2})
self.assertEqual(set(missanc.bases()), {1, 2}) self.assertEqual(missanc.bases(), {1, 2})
self.assertEqual(missanc.missingancestors([3]), [3]) self.assertEqual(missanc.missingancestors([3]), [3])
def testmissingancestorsremove(self): def testmissingancestorsremove(self):
idx = self.parseindex() idx = self.parseindex()
missanc = MissingAncestors(idx, [1]) missanc = MissingAncestors(idx, [1])
revs = {0, 1, 2, 3} revs = {0, 1, 2, 3}
missanc.removeancestorsfrom(revs) missanc.removeancestorsfrom(revs)
self.assertEqual(revs, {2, 3}) self.assertEqual(revs, {2, 3})