Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented Aug 23, 2020

Initial commit.

  • Declared & defined memory_iter()
  • Defined memoryiterobject and a (basic) PyMemoryIter_Type
  • Added the aforementioned memory_iter() to PyMemoryView_Type.tp_iter

linchiwei123 and others added 30 commits July 26, 2020 20:33
... when an unknown option is passed.  TypeError was being raised because a 2to3 fix was missing.

Co-authored-by: Terry Jan Reedy <[email protected]>
The error is exposed on non-UTF-8 locales.
…ation (pythonGH-19969)

(We censor the heck out of actions and some other stuff using a custom "highlighter".)

Co-authored-by: Guido van Rossum <[email protected]>
This will improve the debug experience if something fails in the produced AST. Previously, errors in the produced AST can be felt much later like in the garbage collector or the compiler, making debugging them much more difficult.
Prevent installation on Windows 8 and earlier.
Download UCRT on demand when required (non-updated Windows 8.1 only)
Add reference to py launcher to post-install message
…thonGH-21569)

This consolidates the handling of my_fgets return values, so that interrupts are always handled, even if they come after EOF.

 I believe PyOS_StdioReadline is still buggy in that I/O errors will not result in a proper Python exception being set. However, that is a separate issue.
…honGH-21517)

* Move 'peephole' optimizations into compile.c and perform them directly on the CFG.
…ions and ordering (python#21574)

Also added PEP 585 deprecation notes.
Co-authored-by: Ankit Chandawala <[email protected]>
Co-authored-by: Terry Jan Reedy <[email protected]>
)

My mentee @xvxvxvxvxv noticed that iterating over array.array is
slightly faster than iterating over bytes.  Looking at the source I
observed that arrayiter_next() calls `getitem(ao, it->index++)` wheras
striter_next() uses the idiom (paraphrased)

    item = PyLong_FromLong(seq->ob_sval[it->it_index]);
    if (item != NULL)
        ++it->it_next;
    return item;

I'm not 100% sure but I think that the second version has fewer
opportunity for the CPU to overlap the `index++` operation with the
rest of the code (which in both cases involves a call).  So here I am
optimistically incrementing the index -- if the PyLong_FromLong() call
fails, this will leave the iterator pointing at the next byte, but
honestly I doubt that anyone would seriously consider resuming use of
the iterator after that kind of failure (it would have to be a
MemoryError).  And the author of arrayiter_next() made the same
consideration (or never ever gave it a thought :-).

With this, a loop like

    for _ in b: pass

is now slightly *faster* than the same thing over an equivalent array,
rather than slightly *slower* (in both cases a few percent).
…thonGH-21718)

regrtest_unraisable_hook() temporarily replaces sys.stderr with
sys.__stderr__ to help to display errors when a test captures stderr.
incr cannot be larger than INT_MAX: downcast to int explicitly.
On Windows, fix asyncio recv_into() return value when the socket/pipe
is closed (BrokenPipeError): return 0 rather than an empty byte
string (b'').
@ghost ghost requested a review from gvanrossum as a code owner August 23, 2020 12:51
@gvanrossum
Copy link
Owner

There's something weird in the PR -- I think my master was way behind your master (that's my fault) and you ended up pushing 114 commits by others to the PR. I'm not quite sure how to fix it, even though I have now updated my master. I'll just ignore it, I can see your change just fine.

Copy link
Owner

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Have you run your code yet? I did, and I discovered the missing tp_dealloc that way (well, I had to jump in the debugger for a bit).

BTW have you ran ./configure --with-pydebug? That's important in this phase, the resulting interpreter has many self-checks enabled that will help you debugging your code.

Good luck!

};



Copy link
Owner

Choose a reason for hiding this comment

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

I think three blank lines is too much. :)

Copy link
Author

Choose a reason for hiding this comment

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

Absolutely! Missed that one.

@ghost
Copy link
Author

ghost commented Aug 23, 2020

BTW have you ran ./configure --with-pydebug?

I forgot. Will do it next time before pushing the code.

@gvanrossum gvanrossum marked this pull request as draft August 26, 2020 20:54
Copy link
Owner

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Here are a few suggestions for fixing the coding style per PEP 7. (Read it!)

@ghost
Copy link
Author

ghost commented Aug 31, 2020

Thank you, Guido, for all the suggestions. I will go through PEP7 in detail.

xvxvxvxvxv and others added 4 commits August 31, 2020 20:55
@gvanrossum
Copy link
Owner

Diogo, maybe this would be a good time to try if you can get rid of the 100+ extra commits that somehow got included in this commit. You may have to do some searching about git on stackoverflow.com.

Also, this somehow did register:

Can you come up with more creative commit titles than " Update Objects/memoryobject.c"? If you're committing suggestions from the GitHub UI, you can edit it there. Also, if you're viewing the suggestions from the "Files changed" tab you can use the "Add to batch" option to make a single commit with all the suggestions.

(Don't worry about what you've already done, everybody has to learn these. :-)

@ghost
Copy link
Author

ghost commented Sep 1, 2020

I will look for a solution to get rid of the extra commits, although perhaps a cleaner solution would be for both of us to sync with the official python repo and then I would just push a new branch with all the changes we made so far in one commit. What do you think about this option?

Also, I still want to remove the now double check that unnecessarily happens in memory_item (since we now do the check for dimensions in memoryiter_next).

(Don't worry about what you've already done, everybody has to learn these. :-)

Thank you. Always learning.

@gvanrossum
Copy link
Owner

I synced master fairly recently. (After you pushed this PR though.) If you can pull master from my repo, make a branch off that, and push that branch back and make it into a PR, that should work.

@gvanrossum
Copy link
Owner

Closing in favor of #19.

@gvanrossum gvanrossum closed this Sep 3, 2020
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.