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

all: fix iterable object, implement filter, map, oct and optimise hex #222

Merged
merged 5 commits into from
Jun 8, 2023

Conversation

wetor
Copy link
Contributor

@wetor wetor commented Jun 3, 2023

all: support filter builtin feature and fix iterable object

Python-3.4.9/Doc/c-api/iterator.rst

Python provides two general-purpose iterator objects. The first, a sequence
iterator, works with an arbitrary sequence supporting the :meth:__getitem__
method. The second works with a callable object and a sentinel value, calling
the callable for each item in the sequence, and ending the iteration when the
sentinel value is returned.

Python-3.4.9/Lib/test/test_builtin.py

class Squares:

    def __init__(self, max):
        self.max = max
        self.sofar = []

    def __len__(self): return len(self.sofar)

    def __getitem__(self, i):
        if not 0 <= i < self.max: raise IndexError
        n = len(self.sofar)
        while n <= i:
            self.sofar.append(n*n)
            n += 1
        return self.sofar[i]

In CPython, Squares (5) is an iterable object, and to support this type of iterable object, I made modifications to Iterator. go


Iterator. go

type Iterator struct {
	Pos int
	Seq Object
}
// ...
func (it *Iterator) M__next__() (res Object, err error) {
	if tuple, ok := it.Seq.(Tuple); ok {
		if it.Pos >= len(tuple) {
			return nil, StopIteration
		}
		res = tuple[it.Pos]
		it.Pos++
		return res, nil
	}
	index := Int(it.Pos)
	if I, ok := it.Seq.(I__getitem__); ok {
		res, err = I.M__getitem__(index)
	} else if res, ok, err = TypeCall1(it.Seq, "__getitem__", index); !ok {
		return nil, ExceptionNewf(TypeError, "'%s' object is not iterable", it.Type().Name)
	}
	if err != nil {
		if IsException(IndexError, err) {
			return nil, StopIteration
		}
		return nil, err
	}
	it.Pos++
	return res, nil
}

Corresponding CPython code
Python-3.4.9/Objects/iterobject.c

static PyObject *
iter_iternext(PyObject *iterator)
{
    seqiterobject *it;
    PyObject *seq;
    PyObject *result;

    assert(PySeqIter_Check(iterator));
    it = (seqiterobject *)iterator;
    seq = it->it_seq;
    if (seq == NULL)
        return NULL;
    if (it->it_index == PY_SSIZE_T_MAX) {
        PyErr_SetString(PyExc_OverflowError,
                        "iter index too large");
        return NULL;
    }

    result = PySequence_GetItem(seq, it->it_index);
    if (result != NULL) {
        it->it_index++;
        return result;
    }
    if (PyErr_ExceptionMatches(PyExc_IndexError) ||
        PyErr_ExceptionMatches(PyExc_StopIteration))
    {
        PyErr_Clear();
        Py_DECREF(seq);
        it->it_seq = NULL;
    }
    return NULL;
}

and Iter()

func Iter(self Object) (res Object, err error) {
	if I, ok := self.(I__iter__); ok {
		return I.M__iter__()
	} else if res, ok, err = TypeCall0(self, "__iter__"); ok {
		return res, err
	}
	if ObjectIsSequence(self) {
		return NewIterator(self), nil
	}
	return nil, ExceptionNewf(TypeError, "'%s' object is not iterable", self.Type().Name)
}

Corresponding CPython code
Python-3.4.9/Objects/abstract.c

PyObject *
PyObject_GetIter(PyObject *o)
{
    PyTypeObject *t = o->ob_type;
    getiterfunc f = NULL;
    f = t->tp_iter;
    if (f == NULL) {
        if (PySequence_Check(o))
            return PySeqIter_New(o);
        return type_error("'%.200s' object is not iterable", o);
    }
    else {
        PyObject *res = (*f)(o);
        if (res != NULL && !PyIter_Check(res)) {
            PyErr_Format(PyExc_TypeError,
                         "iter() returned non-iterator "
                         "of type '%.100s'",
                         res->ob_type->tp_name);
            Py_DECREF(res);
            res = NULL;
        }
        return res;
    }
}

Finally, filter and map methods were fully implemented

py/tests/filter.py is a copy of Python-3.4.9/Lib/test/test_builtin.py:BuiltinTest.test_filter()
and
py/tests/map.py is a copy of Python-3.4.9/Lib/test/test_builtin.py:BuiltinTest.test_map()

builtin: Implement oct and optimise hex

Through benchmark testing, the speed of hex has indeed been improved

My English is not very good, so forgive me for not providing sufficient explanations. Welcome to ask questions here

@codecov
Copy link

codecov bot commented Jun 3, 2023

Codecov Report

Patch coverage: 83.13% and project coverage change: +0.10 🎉

Comparison is base (acd458b) 74.42% compared to head (04a9c5b) 74.52%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #222      +/-   ##
==========================================
+ Coverage   74.42%   74.52%   +0.10%     
==========================================
  Files          76       78       +2     
  Lines       12675    12804     +129     
==========================================
+ Hits         9433     9542     +109     
- Misses       2567     2583      +16     
- Partials      675      679       +4     
Impacted Files Coverage Δ
py/arithmetic.go 61.04% <ø> (-0.18%) ⬇️
py/list.go 64.16% <60.00%> (+0.03%) ⬆️
stdlib/builtin/builtin.go 78.21% <63.79%> (-1.86%) ⬇️
py/iterator.go 86.95% <88.88%> (-3.96%) ⬇️
py/filter.go 92.59% <92.59%> (ø)
py/object.go 89.18% <93.93%> (+22.52%) ⬆️
py/dict.go 82.01% <100.00%> (ø)
py/exception.go 64.00% <100.00%> (+3.79%) ⬆️
py/internal.go 57.14% <100.00%> (+1.58%) ⬆️
py/map.go 100.00% <100.00%> (ø)
... and 1 more

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@sbinet sbinet left a comment

Choose a reason for hiding this comment

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

thanks for the PR.

I have mostly cosmetics comments, see below.

py/filter.go Outdated Show resolved Hide resolved
py/map.go Outdated Show resolved Hide resolved
py/object.go Outdated Show resolved Hide resolved
py/object.go Outdated Show resolved Hide resolved
py/object.go Outdated Show resolved Hide resolved
py/object.go Outdated Show resolved Hide resolved
@wetor wetor force-pushed the main branch 3 times, most recently from 4979c19 to b184a43 Compare June 6, 2023 03:34
Copy link
Member

@sbinet sbinet left a comment

Choose a reason for hiding this comment

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

LGTM

could you split the commits between implementing filter, implementing map, implementing oct and optimising hex ?
(it's good practice and brings easier to understand git-history)
if not, I'll do it, once @ncw chimed in about ObjectIsTrue.

@wetor
Copy link
Contributor Author

wetor commented Jun 7, 2023

Of course, I will split commit later

@wetor wetor force-pushed the main branch 3 times, most recently from f364ef9 to 8681126 Compare June 7, 2023 11:52
@wetor
Copy link
Contributor Author

wetor commented Jun 8, 2023

I found that the basic types (int, string, etc.) did not run Ready() correctly, resulting some attribute was not registered correctly and was fixed by the way
b4b7ef2

Copy link
Member

@sbinet sbinet left a comment

Choose a reason for hiding this comment

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

LGTM, modulo a very minor nit.

thanks again.

py/object.go Outdated Show resolved Hide resolved
Copy link
Member

@sbinet sbinet left a comment

Choose a reason for hiding this comment

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

LGTM.

thanks again.

@sbinet sbinet merged commit 7102b79 into go-python:main Jun 8, 2023
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.

2 participants