dfurtado / dataclass-csv

Map CSV to Data Classes
Other
194 stars 20 forks source link

Default values are passed to their constructor #32

Open tewe opened 4 years ago

tewe commented 4 years ago

This generalises #20.

I completed the example from the README. The following works.

import dataclasses
import io
import re

from dataclass_csv import DataclassReader

class SSN:
    def __init__(self, val):
        if re.match(r"\d{9}", val):
            self.val = f"{val[0:3]}-{val[3:5]}-{val[5:9]}"
        elif re.match(r"\d{3}-\d{2}-\d{4}", val):
            self.val = val
        else:
            raise ValueError(f"Invalid SSN: {val!r}")

@dataclasses.dataclass
class User:
    name: str
    ssn: SSN

print(list(DataclassReader(io.StringIO("name,ssn\nAlice,123456789"), User)))

After replacing the SSN in the data with a default value, the error below occurs.

class User:
    name: str
    ssn: SSN = SSN("123456789")

print(list(DataclassReader(io.StringIO("name,ssn\nAlice,"), User)))
  File "readme.py", line 10, in __init__
    if re.match(r"\d{9}", val):
TypeError: expected string or bytes-like object

The reason being that the SSN constructor is called with the default value, which is an object. The next snippet works around that.

class User:
    name: str
    ssn: SSN = "123456789"

But now this class can no longer be instantiated directly without creating broken objects, and type checkers will complain.

I think if a default value exists it should be used as is, and not passed to the constructor of its class.

qpwo commented 3 years ago

Replicated

from dataclasses import dataclass
from io import StringIO
from dataclass_csv import DataclassReader
class C:
    def __init__(self, x):
        print(f"initialized with {x!r}")
        self.x = x
@dataclass
class Row:
    b: int
    c: C = C(5)
# initialized with 5
A = list(DataclassReader(StringIO("b,c\n1,"), Row))
# initialized with <__main__.C object at 0x10385ea90>
print(A[0].c.x)
# <__main__.C at 0x10385ea90>
dfurtado commented 3 years ago

Hi @qpwo

Thanks for reporting the issue.

I started looking into it a few days ago and a fix for it should be available very soon.

//Daniel