Skip to content

Session.virtualfile_to_dataset: Add new parameters 'dtype'/'index_col' for pandas output - #3140

Merged
seisman merged 7 commits into
mainfrom
dataset/to_dataframe
Mar 27, 2024
Merged

Session.virtualfile_to_dataset: Add new parameters 'dtype'/'index_col' for pandas output#3140
seisman merged 7 commits into
mainfrom
dataset/to_dataframe

Conversation

@seisman

@seismanseisman commented Mar 26, 2024

Copy link
Copy Markdown
Member

Address #3131 (comment).

Changes in this PR:

  • GMT_DATASET.to_dataframe: Add new parameters names/dtype/index_col
  • Session.virtualfile_to_dataset: AAdd new parameter dtype/index_col and rename column_names to names
  • Simplify some wrappers

The parameter names names/dtype/index_col are very close to the ones in the pd.read_csv function.

Comment threadpygmt/clib/session.py
@@ -1747,6 +1747,8 @@ def virtualfile_to_dataset(
vfname: str,
output_type: Literal["pandas", "numpy", "file"] = "pandas",
column_names: list[str] | None = None,

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

In Session.virtualfile_to_dataset, do we want to rename column_names to names?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Done in f211b7d.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I kinda prefer column_names, we don't necessarily need to follow pd.read_csv here.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Do you mean using column_names in both GMT_DATASET.to_dataframe and Session.virtualfile_to_dataset?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, i.e. revert f211b7d

@seismanseisman changed the title GMT_DATASET.to_dataframe: Add the parameter 'names' for specifying the column namesGMT_DATASET.to_dataframe: Add the parameter 'names'/'dtype'/'index_col'Mar 26, 2024
@seisman
seisman marked this pull request as ready for review March 26, 2024 06:55
@seismanseisman added enhancement Improving an existing feature needs review This PR has higher priority and needs review. labels Mar 26, 2024
@seismanseisman added this to the 0.12.0 milestone Mar 26, 2024
@seismanseisman changed the title GMT_DATASET.to_dataframe: Add the parameter 'names'/'dtype'/'index_col'GMT_DATASET.to_dataframe: Add new parameters 'names'/'dtype'/'index_col'Mar 26, 2024
@seisman
seismanforce-pushed the dataset/to_dataframe branch from 9f67844 to 4554e63CompareMarch 26, 2024 07:03
@seismanseisman changed the title GMT_DATASET.to_dataframe: Add new parameters 'names'/'dtype'/'index_col'Session.virtualfile_to_dataset: Add new parameters 'dtype'/'index_col' and rename 'column_names' to 'names'Mar 26, 2024
Comment threadpygmt/datatypes/dataset.py Outdated
@seisman
seismanforce-pushed the dataset/to_dataframe branch from f11dc89 to abda146CompareMarch 27, 2024 02:02
@seismanseisman changed the title Session.virtualfile_to_dataset: Add new parameters 'dtype'/'index_col' and rename 'column_names' to 'names'Session.virtualfile_to_dataset: Add new parameters 'dtype'/'index_col'Mar 27, 2024
@seismanseisman changed the title Session.virtualfile_to_dataset: Add new parameters 'dtype'/'index_col'Session.virtualfile_to_dataset: Add new parameters 'dtype'/'index_col' for pandas outputMar 27, 2024
@seisman
seisman merged commit f0d4d21 into mainMar 27, 2024
@seisman
seisman deleted the dataset/to_dataframe branch March 27, 2024 02:27
@seismanseisman removed the needs review This PR has higher priority and needs review. label Mar 27, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementImproving an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@seisman@weiji14@yvonnefroehlich