GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — all: fix iterable object, implement `filter`, `map`, `oct` and optimise `hex` by wetor · Pull Request #222 · go-python/gpython

Conversation

@wetor

@wetor wetor commented Jun 3, 2023

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@codecov

codecov Bot commented Jun 3, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

sbinet
sbinet reviewed Jun 5, 2023
View reviewed changes

@sbinet sbinet left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread py/filter.go Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread py/map.go Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread py/object.go Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread py/object.go Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread py/object.go Resolved

Uh oh!

There was an error while loading. Please reload this page.

Comment thread py/object.go Resolved

Uh oh!

There was an error while loading. Please reload this page.

@wetor
wetor force-pushed the main branch 3 times, most recently from 4979c19 to b184a43 Compare June 6, 2023 03:34
sbinet
sbinet approved these changes Jun 7, 2023
View reviewed changes

@sbinet sbinet left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@wetor
wetor force-pushed the main branch from b184a43 to 1b2b0a2 Compare June 7, 2023 10:15
@wetor wetor closed this Jun 7, 2023
@wetor wetor reopened this Jun 7, 2023
@wetor

wetor commented Jun 7, 2023

Copy link
Copy Markdown
Contributor Author

Of course, I will split commit later

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

wetor commented Jun 8, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

sbinet
sbinet approved these changes Jun 8, 2023
View reviewed changes

@sbinet sbinet left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread py/object.go Resolved

Uh oh!

There was an error while loading. Please reload this page.

wetor added 5 commits June 8, 2023 17:36
@wetor
py: fix iterable object
dc25779
@wetor
py: implement filter
2efe52e
@wetor
py: implement map
487fbd9
@wetor
builtin: implement oct and optimise hex
cc7dfae
@wetor
py: fix basic type not run Ready()
04a9c5b
@wetor
wetor force-pushed the main branch from b4b7ef2 to 04a9c5b Compare June 8, 2023 09:36
sbinet
sbinet approved these changes Jun 8, 2023
View reviewed changes

@sbinet sbinet left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM.

thanks again.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@sbinet
sbinet merged commit 7102b79 into go-python:main Jun 8, 2023
@drew-512 drew-512 mentioned this pull request Nov 10, 2026
vm: use truthiness of the __exit__ result in WITH_CLEANUP #269
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sbinet sbinet sbinet approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@wetor @sbinet
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.