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

Restify PEP 307 #229

Merged
merged 5 commits into from
Mar 27, 2017
Merged

Restify PEP 307 #229

merged 5 commits into from
Mar 27, 2017

Conversation

serhiy-storchaka
Copy link
Member

No description provided.

@serhiy-storchaka
Copy link
Member Author

For #4.

pep-0307.txt Outdated
x = C()
x.foo = 42
print len(pickle.dumps(x, 1))
.. code-block:: python
Copy link
Member

Choose a reason for hiding this comment

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

I think this directive is the cause of the Travis failures. Updating Travis to install pygments wouldn't be hard, but do we know if python.org has it installed? @berkerpeksag , do you know?

Copy link
Member

Choose a reason for hiding this comment

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

I think @ilevkivskyi's PR at pythondotorg repo is meant to address this python/pythondotorg#1063

I haven't had chance to review / test it locally ..

Copy link
Member

Choose a reason for hiding this comment

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

IIRC, with pygments installed, docutils produces HTML that is correctly rendered by python.org without any changes to pep2html script (although still b/w). My PR adds little post-processing and CSS styles to actually make it colored on python.org

Copy link
Member

Choose a reason for hiding this comment

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

So is it safe to update the Travis check to assume that pygments is installed on python.org for rendering the PEPs?

Copy link
Member

Choose a reason for hiding this comment

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

So is it safe to update the Travis check to assume that pygments is installed on python.org for rendering the PEPs?

It is safe if pygments are actually installed on the machine were pep2html runs, I don't know if this is the case (my PR on pythondotorg adds pygments to requirements.txt but it is not merged yet).

@serhiy-storchaka
Copy link
Member Author

If there are problems with this I can remove code-block directives.

@brettcannon
Copy link
Member

@serhiy-storchaka why don't we remove them for now so this is no longer blocked; once Pygments is on www.python.org -- or we get the PEPs hosted on Read the Docs: python/core-workflow#5 -- we can add it back.

pep-0307.txt Outdated
Pickling new-style objects causes serious pickle bloat. For
example,
Pickling new-style objects causes serious pickle bloat. For
example, ::
Copy link
Member

Choose a reason for hiding this comment

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

I'd change this to "For example::".

pep-0307.txt Outdated

There are several APIs that a class can use to control pickling.
Perhaps the most popular of these are ``__getstate__`` and
``__setstate__; but the most powerful one is ``__reduce__``. (There's
Copy link
Member

Choose a reason for hiding this comment

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

missing ``

pep-0307.txt Outdated
return values are interpreted exactly the same, though, and we'll
refer to these collectively as ``__reduce__``.

IMPORTANT: pickling of classic class instances does not look for a
Copy link
Member

Choose a reason for hiding this comment

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

you can change this to **Important:** since we now have text formatting.

pep-0307.txt Outdated
``__reduce_ex__`` method will be called with a single integer
argument, the protocol version.

The 'object' class implements both ``__reduce__`` and ``__reduce_ex__;
Copy link
Member

Choose a reason for hiding this comment

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

missing ``

pep-0307.txt Outdated

- First, it tries to call ``self.__dict__.update(state)``.

- If the update() call fails with a RuntimeError exception, it
Copy link
Member

Choose a reason for hiding this comment

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

You should probably wrap update(), RuntimeError, setattr(...) and (key, value) with backticks in order to be consistent with the rest of the file.

pep-0307.txt Outdated
``__reduce__`` implementation (``copy_reg._reduce``) works as follows:

Let ``D`` be the class on the object to be pickled. First, find the
nearest base class that is implemented in ``C`` (either as a
Copy link
Member

Choose a reason for hiding this comment

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

I think C here is the programming language, not a class named C.

pep-0307.txt Outdated
256 MAX MAX Reserved for future assignment
===== ==== ===== =================================================

MAX stands for 2147483647, or ``2**31-1``. This is a hard limitation
Copy link
Member

Choose a reason for hiding this comment

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

You can probably use backticks for constants like MAX.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is not a Python or C constant or expression, this is just a metavariable. I think emphasizing (*MAX*) would look better.

@brettcannon brettcannon merged commit a3062a8 into master Mar 27, 2017
@brettcannon brettcannon deleted the pep-0307-rest branch March 27, 2017 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants