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

Added the 'placeholder' write_csv func, fixes #1542 #1603

Merged
merged 4 commits into from
Dec 1, 2021

Conversation

jmao-denver
Copy link
Contributor

A write_csv func with miniaml number of params as a temporary fillgap
for future full-featured implementation

Comment on lines 92 to 95
def write_csv(self: Table, path: str, cols: List[str] = []) -> None:
try:
_JTableTools.writeCsv(self._j_table, path, *cols)
except Exception as e:
raise DHError("write csv failed.") from e

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want this method on the table. We support many data formats and have finally started packaging each format into its own module. By having this method here, we have decided that CSV is somehow more special than the other formats, and we are willing to add it to the table API. I think the method in the CSV module is sufficient. If it isn't, we can easily add a method here in the future. It is harder to take a method away from here.

@jmao-denver jmao-denver merged commit d55c7d9 into deephaven:main Dec 1, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Dec 1, 2021
@jmao-denver jmao-denver deleted the f-1542 branch February 8, 2023 18:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants