sdv-dev / SDV

Synthetic data generation for tabular data
https://docs.sdv.dev/sdv
Other
2.23k stars 293 forks source link

Documentation suggests "path" keyword argument in save method #1344

Closed arvkevi closed 1 year ago

arvkevi commented 1 year ago

Environment Details

Please indicate the following details about the environment in which you found the bug:

Error Description

The documentation suggests there is a path keyword argument in the save method, but the save method of the BaseSynthesizer class does not have a keyword argument.

Steps to reproduce

import numpy as np
import pandas as pd

from sdv.metadata import SingleTableMetadata
from sdv.single_table import CTGANSynthesizer

a = pd.DataFrame(np.random.randint(0, 1, (100, 10)), columns=list("abcdefghij"))

metadata = SingleTableMetadata()
metadata.detect_from_dataframe(data=a)

synthesizer = CTGANSynthesizer(
    metadata,
)
synthesizer.fit(a)
synthesizer.save(path="my_synthesizer.pkl")

This raised a TypeError:

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
[/var/folders/g2/b2m1qtks3_j97bl9ttnkrssw0000gp/T/ipykernel_89741/1919952502.py](https://file+.vscode-resource.vscode-cdn.net/var/folders/g2/b2m1qtks3_j97bl9ttnkrssw0000gp/T/ipykernel_89741/1919952502.py) in ()
     15 )
     16 synthesizer.fit(a)
---> 17 synthesizer.save(path="my_synthesizer.pkl")

TypeError: save() got an unexpected keyword argument 'path'

Passing a string as an argument works as expected:

synthesizer.save("my_synthesizer.pkl")

I'm happy to open a PR, provided some guidance, and if you think it would be helpful. Congrats on the v1.0 release btw, I love this library!

npatki commented 1 year ago

Hi @arvkevi thanks for catching this and providing the details.

This is a mistake in the docs. The correct parameter name should be filepath instead of path, so the following should work.

synthesizer.save(filepath='my_synthesizer.pkl')

And calling it by the keyword name is indeed optional. So as you mention, you can always call save without it.

I've updated the docs now (may require a refresh to see). Seems like all our synthesizers accidentally had path instead of filepath.

image
arvkevi commented 1 year ago

Thanks, @npatki. Your suggestion worked on my end and the save docs for all synthesizers read "filepath" now.

npatki commented 1 year ago

Great thanks for confirming 😃 Closing off the issue!