Don't subclass

PyO3/pyo3#220 is not fixed yet.

.pre-commit-config.yaml

@@ -1,6 +1,13 @@
 repos:
--   repo: https://github.com/ambv/black
+  - repo: https://github.com/ambv/black
     rev: stable
     hooks:
     - id: black
       language_version: python3.6
+  - repo: local
+    hooks:
+      - id: cargo-fmt
+        name: cargo-fmt
+        language: system
+        pass_filenames: false
+        entry: cargo fmt

Cargo.toml

@@ -3,7 +3,6 @@ name = "rustcsv"
 version = "0.1.0"
 authors = ["Joar Wandborg <joar@wandborg.se>"]
 build = "build.rs"
-publish = false
 
 [build-dependencies]
 built = "^0.3"
@@ -16,12 +15,9 @@ env_logger = "*"
 tempfile = "3"
 
 [dependencies.pyo3]
-version = "0.5.0"
+version = "0.5.0-pre"
 features = ["extension-module"]
-git = "https://github.com/PyO3/pyo3"
-# Commit:
-# Fix cfg on PyEval_InitThread to fix #219
-rev = "10ef6cd1113c5d8fbcea01481babf1d327c2bbd6"
+path = "pyo3"
 
 [profile.release]
 lto = true

rustcsv/__init__.py

@@ -12,32 +12,32 @@
 
 __all__ = ["CSVReader", "CSVWriter", "__build__", "version"]
 
-
+CSVReader = _RustCSVReader
 # Can't subclass for better docstrings: https://github.com/PyO3/pyo3/issues/220
-class CSVReader(_RustCSVReader):
-    def __new__(
-        cls,
-        source: Union[BinaryIO, str],
-        delimiter: bytes = b",",
-        terminator: bytes = b"\n",
-    ):
-        """
-
-        Parameters
-        ----------
-        source
-            :any:`binary file` or string to read CSV from.
-        delimiter
-            Byte to use as CSV field delimiter
-        terminator
-            Byte to use as CSV record terminator
-
-        Returns
-        -------
-        CSVReader
-        """
-        self = super(CSVReader, cls).__new__(
-            cls, path_or_fd=source, delimiter=delimiter, terminator=terminator
-        )
-
-        return self
+# class CSVReader(_RustCSVReader):
+#     def __new__(
+#         cls,
+#         source: Union[BinaryIO, str],
+#         delimiter: bytes = b",",
+#         terminator: bytes = b"\n",
+#     ):
+#         """
+#
+#         Parameters
+#         ----------
+#         source
+#             :any:`binary file` or string to read CSV from.
+#         delimiter
+#             Byte to use as CSV field delimiter
+#         terminator
+#             Byte to use as CSV record terminator
+#
+#         Returns
+#         -------
+#         CSVReader
+#         """
+#         self = super(CSVReader, cls).__new__(
+#             cls, path_or_fd=source, delimiter=delimiter, terminator=terminator
+#         )
+#
+#         return self

src/lib.rs

@@ -1,14 +1,13 @@
-#![feature(specialization, extern_prelude, core_intrinsics)]
+#![feature(specialization)]
 
 extern crate built;
 extern crate csv;
 extern crate env_logger;
 #[macro_use]
 extern crate log;
+extern crate tempfile;
 #[macro_use]
 extern crate pyo3;
-// Used for testing
-extern crate tempfile;
 
 pub mod py_file;
 pub mod reader;
@@ -24,7 +23,7 @@ pub mod built_info {
 macro_rules! pyo3_built {
     ($py: ident, $info: ident) => {{
         use $crate::built::util::strptime;
-        use $crate::pyo3::prelude::PyDict;
+        use $crate::pyo3::types::{PyDict, PyString};
 
         let info = PyDict::new($py);
 
@@ -85,7 +84,7 @@ macro_rules! pyo3_built {
 // N.B: names: "_rustcsv" must be the name of the `.so` or `.pyd` file
 /// PyO3 + rust-csv
 /// An exploration in reading CSV as fast as possible from Python.
-#[pymodinit(_rustcsv)]
+#[pymodule(_rustcsv)]
 pub fn rustcsv(_py: Python, m: &PyModule) -> PyResult<()> {
     use built_info;
     env_logger::init();

src/py_file.rs

@@ -1,6 +1,11 @@
 extern crate pyo3;
 
+use pyo3::exceptions as exc;
 use pyo3::prelude::*;
+use pyo3::types::PyBytes;
+use pyo3::types::PyObjectRef;
+use pyo3::types::PyString;
+use pyo3::FromPyObject;
 use pyo3::Python;
 use std::io;
 use std::io::Read;
@@ -65,15 +70,17 @@ impl PyFile {
         match call_result.extract(py) {
             Ok(r) => Ok(Box::new(r)),
             //
-            Err(error) => if py.is_instance::<PyString, _>(call_result.as_ref(py))? {
-                return Err(exc::TypeError::py_err(format!(
-                    "The file {:?} is not open in binary mode. (Cause: {:?})",
-                    self.file_like.as_ref(py),
-                    error.to_object(py).as_ref(py),
-                )));
-            } else {
-                return Err(error);
-            },
+            Err(error) => {
+                if py.is_instance::<PyString, _>(call_result.as_ref(py))? {
+                    return Err(exc::TypeError::py_err(format!(
+                        "The file {:?} is not open in binary mode. (Cause: {:?})",
+                        self.file_like.as_ref(py),
+                        error.to_object(py).as_ref(py),
+                    )));
+                } else {
+                    return Err(error);
+                }
+            }
         }
     }
 
@@ -163,7 +170,8 @@ impl<'source> FromPyObject<'source> for PyFile {
 mod tests {
     use super::PyFile;
     use pyo3::prelude::*;
-    use pyo3::{PyDict, PyResult, Python};
+    use pyo3::types::PyDict;
+    use pyo3::{PyResult, Python};
     use std::io::Read;
     use std::io::Write;
     use tempfile::NamedTempFile;

src/reader.rs

@@ -1,6 +1,15 @@
 extern crate pyo3;
 use py_file::PyFile;
+use pyo3::class::PyIterProtocol;
+use pyo3::exceptions as exc;
 use pyo3::prelude::*;
+use pyo3::types::PyBytes;
+use pyo3::types::PyObjectRef;
+use pyo3::types::PyString;
+use pyo3::PyObject;
+use pyo3::PyRawObject;
+use pyo3::PyResult;
+use pyo3::Python;
 use record;
 use util::get_optional_single_byte;
 
@@ -14,10 +23,11 @@ pub enum CSVSource {
     Readable(PyFile),
 }
 
-/// The `CSVReader` Python class
+// Python docstring for CSVReader
+/// CSVReader(path_or_fd, delimiter, terminator)
+/// --
 #[pyclass(subclass)]
 pub struct CSVReader {
-    token: PyToken,
     // It would be nice to have a reference to csv::Reader here,
     // but I haven't figured out lifetimes yet.
     /// Iterator over the parsed records
@@ -61,6 +71,8 @@ pub fn make_records_iterator(
 /// Implements the Python type methods for `CSVReader`
 #[pymethods]
 impl CSVReader {
+    /// CSVReader(path_or_fd, delimiter: bytes, terminator: bytes)
+    /// --
     /// Creates a new CSVReader instance
     ///
     /// - `path_or_fd` - Either a string path to a file or a [binary file].
@@ -77,14 +89,12 @@ impl CSVReader {
         path_or_fd: &'static PyObjectRef,
         delimiter: Option<&PyBytes>,
         terminator: Option<&PyBytes>,
+        py: Python,
     ) -> PyResult<()> {
         debug!(
             "__new__: path_or_fd: {:?}, delimiter: {:?}, terminator: {:?}",
             path_or_fd, delimiter, terminator
         );
-        let gil = Python::acquire_gil();
-        let py = gil.python();
-
         let delimiter_arg = get_optional_single_byte(delimiter, b',')?;
         let terminator_arg = get_optional_single_byte(terminator, b'\n')?;
 
@@ -99,7 +109,7 @@ impl CSVReader {
         };
 
         match make_records_iterator(source, delimiter_arg, terminator_arg) {
-            Ok(iter) => obj.init(|token| CSVReader { token, iter }),
+            Ok(iter) => obj.init(|| CSVReader { iter }),
             Err(error) => match error.into_kind() {
                 csv::ErrorKind::Io(err) => Err(err.into()),
                 err => Err(exc::IOError::py_err(format!(
@@ -138,10 +148,10 @@ impl PyIterProtocol for CSVReader {
         match self.iter.next() {
             Some(res) => match res {
                 Ok(r) => {
-                    let py = self.token.py();
+                    let gil = Python::acquire_gil();
+                    let py = gil.python();
                     let rec: record::Record = r.into();
-                    let t = rec.into_tuple(py);
-                    Ok(Some(t.into_object(py)))
+                    Ok(Some(rec.into_object(py)))
                 }
                 Err(error) => match error.into_kind() {
                     csv::ErrorKind::Io(err) => {

src/record.rs

@@ -1,7 +1,12 @@
 extern crate csv;
 extern crate pyo3;
 
-use pyo3::prelude::*;
+use pyo3::types::PyTuple;
+use pyo3::IntoPyObject;
+use pyo3::IntoPyTuple;
+use pyo3::Py;
+use pyo3::PyObject;
+use pyo3::Python;
 use std::convert;
 
 pub struct Record {
@@ -14,9 +19,15 @@ impl convert::From<csv::StringRecord> for Record {
     }
 }
 
+impl IntoPyObject for Record {
+    fn into_object(self, py: Python) -> PyObject {
+        self.into_tuple(py).into()
+    }
+}
+
 impl IntoPyTuple for Record {
     fn into_tuple(self, py: Python) -> Py<PyTuple> {
         let items: Vec<&str> = self.r.iter().collect();
-        PyTuple::new(py, items.as_slice())
+        PyTuple::new(py, items)
     }
 }

src/util.rs

@@ -1,5 +1,7 @@
 extern crate pyo3;
+use pyo3::exceptions as exc;
 use pyo3::prelude::*;
+use pyo3::types::PyBytes;
 
 pub fn get_optional_single_byte(bytes: Option<&PyBytes>, default: u8) -> PyResult<u8> {
     match bytes {
@@ -11,7 +13,7 @@ pub fn get_optional_single_byte(bytes: Option<&PyBytes>, default: u8) -> PyResul
 /// Extracts a single u8 from a PyBytes object
 /// If the PyBytes object contains more or less than 1 byte, an error is returned.
 pub fn get_single_byte(bytes: &PyBytes) -> PyResult<u8> {
-    let data: &[u8] = bytes.data();
+    let data: &[u8] = bytes.as_bytes();
     if data.len() > 1 {
         error!("data is too long: {:?}", data);
         return Err(PyErr::new::<exc::ValueError, _>((format!(

src/writer.rs

@@ -1,19 +1,23 @@
 extern crate csv;
 extern crate pyo3;
 
-use pyo3::prelude::*;
+use pyo3::prelude::{pyclass, pymethods};
 
 use py_file::PyFile;
+use pyo3::exceptions as exc;
+use pyo3::types::PyBytes;
+use pyo3::types::PyObjectRef;
+use pyo3::types::PyTuple;
+use pyo3::FromPyObject;
+use pyo3::ObjectProtocol;
+use pyo3::PyRawObject;
+use pyo3::PyResult;
+use pyo3::PyTryFrom;
+use pyo3::Python;
 use util::get_optional_single_byte;
 
-#[pyclass(subclass)]
-pub struct QuoteStyle {
-    token: PyToken,
-}
-
 #[pyclass(subclass)]
 pub struct CSVWriter {
-    token: PyToken,
     writer: csv::Writer<PyFile>,
 }
 
@@ -45,16 +49,17 @@ impl CSVWriter {
             .double_quote(double_quote.unwrap_or(true))
             .terminator(csv::Terminator::Any(get_optional_single_byte(
                 terminator, b'\n',
-            )?)).escape(get_optional_single_byte(escape, b'\\')?)
+            )?))
+            .escape(get_optional_single_byte(escape, b'\\')?)
             .quote_style(parse_quote_style(
                 quote_style.unwrap_or("necessary".into()).as_str(),
-            )?).from_writer(PyFile::extract(fd)?);
-        obj.init(|token| CSVWriter { writer, token })
+            )?)
+            .from_writer(PyFile::extract(fd)?);
+        obj.init(|| CSVWriter { writer })
     }
 
     /// Writes a CSV row to the file.
-    fn writerow(&mut self, record: &PyObjectRef) -> PyResult<()> {
-        let py = self.token.py();
+    fn writerow(&mut self, record: &PyObjectRef, py: Python) -> PyResult<()> {
         if !py.is_instance::<PyTuple, PyObjectRef>(record)? {
             return Err(exc::TypeError::py_err(format!(
                 "Expected tuple, got {:?}",

travis/install-rust.sh

@@ -17,15 +17,15 @@ install_rust() {
             | sh -s -- -y --default-toolchain "$RUST_VERSION"
     fi
     if ! grep "$CARGO_BIN" <<<"$PATH" &> /dev/null; then
-        green "Addigng $CARGO_BIN to \$PATH"
+        green "Adding $CARGO_BIN to \$PATH"
         export PATH="$CARGO_BIN:$PATH"
     fi
 }
 
 
 if [[ "${BASH_SOURCE[0]}" = "${0}" ]]; then
     red "The script should be sourced so that it's able to export \$PATH." \
-        "e.g. 'source ${BASH_SOURCE[@]}' will install rust and re-export \$PATH"
+        "e.g. 'source ${BASH_SOURCE[*]}' will install rust and re-export \$PATH"
     exit 1
 else
     install_rust "$@"