bpo-34533: Apply PEP384 to _csv module #8977
Conversation
|
You should ensure the build actually succeeds and the tests pass before you submit a PR (see also the developer's guide). Be aware that I'm preparing to submit a PR applying this refactoring to the _ssl module (based on the old patch at https://bugs.python.org/issue15670). Also, the issue that you created on the tracker happens to be a duplicate of https://bugs.python.org/issue14935. |
|
@ZackerySpytz Sorry about that, contbuild is passing now! There was an incorrect assumption on one of the csv modules test. Given that Dialect class is a HeapType now, it should be able to be pickled when proto == 0 or 1. Also, good point. I didn't see that previous issue. Given that the other one has been there for 6+ years, we can probably just take this one :) |
Modules/_csv.c
Outdated
| @@ -45,9 +45,10 @@ _csv_free(void *m) | |||
| _csv_clear((PyObject *)m); | |||
| } | |||
|
|
|||
| static struct PyModuleDef _csvmodule; | |||
| static struct PyModuleDef _csvmodule_def; | |||
| static PyObject *_csvmodule; | |||
There was a problem hiding this comment.
Do we need this at file scope? Given that this is replacing PyObject *module at line 1618
There was a problem hiding this comment.
_csvmodule_def is required in _csvstate_global. But you are right, we don't need _csvmodule at file scope. Will fix.
https://bugs.python.org/issue34533