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

What is the purpose of 'ParquetFile.sep' attribute? #687

Closed
yohplala opened this issue Sep 29, 2021 · 5 comments · May be fixed by #799
Closed

What is the purpose of 'ParquetFile.sep' attribute? #687

yohplala opened this issue Sep 29, 2021 · 5 comments · May be fixed by #799

Comments

@yohplala
Copy link

Hi,
Currently investigating fastparquet source code, I could notice the undocumented sep attribute in ParquetFile class.

It appears in __init__ function signature, line 96.

    def __init__(self, fn, verify=False, open_with=default_open,
                 root=False, sep=None, fs=None, pandas_nulls=True):

It is recorded line 161
self.sep = sep

It can be displayed through __getstate__ line 162.

    def __getstate__(self):
        return {"fn": self.fn, "open": self.open, "sep": self.sep, "fmd": self.fmd,
                "pandas_nulls": self.pandas_nulls}

But then i don't see it being used elsewhere.
Please, what is its purpose?

Thanks for your feedback,
Have a good day,
Bests

@yohplala yohplala changed the title What is the purpose of 'Parquetfile.sep' attribute? What is the purpose of 'ParquetFile.sep' attribute? Sep 29, 2021
@martindurant
Copy link
Member

Probably for creating path names from the relative paths contained in the metadata. However, even Windows works with a separator of "/", and fsspec filesystems all use that, so it probably went defunct. The signature predates fsspec (and you can now explicitly pass a filesystem object).

@yohplala
Copy link
Author

Ok, I propose to solve this ticket when corresponding dask ticket is solved.

@martindurant
Copy link
Member

The linked issue is closed, but this thread is now very old.

@yohplala
Copy link
Author

The linked issue is closed, but this thread is now very old.

Yes, sorry, there has been no follow-up so far.
I propose to close with PR #799: all dask tests are now running.

@martindurant
Copy link
Member

Actually, let's just close this. We probably should leave the unused parameter in place for at least another release cycle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants