Skip to content

[GAIA-21588] Create a function to convert v2prime result into dataframe #379

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

Merged
merged 14 commits into from
Jun 16, 2023

Conversation

muzigao-gro
Copy link
Contributor

See this discussion: https://gro-intelligence.slack.com/archives/CUVUBHQE9/p1683578154127809?thread_ts=1683568514.990159&cid=CUVUBHQE9

I'm still debating whether or not we want to make dataframe as the default response format for get_data_points, but now I'm making a separate function:

  • converted timestamp into date (so it's human readable)
  • merge series_description into combined dataframe

@sahuguet
Copy link
Contributor

sahuguet commented May 11, 2023 via email

@muzigao-gro
Copy link
Contributor Author

This is a REST API, not a pandas endpoint. We should support standards like CSV or JSON.
@sahuguet what do you mean? can you elaborate that?

@muzigao-gro muzigao-gro requested a review from elia-gro June 1, 2023 15:29
Copy link

@elia-gro elia-gro left a comment

Choose a reason for hiding this comment

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

LGTM on the output and conversion of pandas DataFrame

}
},
{
"data_points": [
Copy link

Choose a reason for hiding this comment

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

Can we have more than one data points?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

data_points is the list of data points per series, yes you can have multiple series in response, but each series dic should only have one data_points attribute

mock_get_data_points.return_value = mock_v2_prime_data_response.copy()
df = self.client.get_data_points_df(**mock_v2_prime_data_request)

self.assertEqual(df.iloc[0]["start_timestamp"], date(2023, 5, 1))
Copy link
Contributor

Choose a reason for hiding this comment

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

Can use pandas.testing assert_frames_equal, and init expected df directly so that it's easier to read:
https://github.com/gro-intelligence/gro/blob/main/infrastructure/derived-compute/historical_stats/test_generate_10y_avg.py#L28

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ha that's a good idea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated!

return data_stream_list


def get_data_points_df(self, **selections):
Copy link
Contributor

Choose a reason for hiding this comment

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

Add type annotations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

Copy link
Contributor

@John-Desmond John-Desmond left a comment

Choose a reason for hiding this comment

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

minor updates requested, otherwise LGTM

Copy link
Contributor

@HonghuiLu2021 HonghuiLu2021 left a comment

Choose a reason for hiding this comment

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

LGTM!

@muzigao-gro muzigao-gro merged commit d0b84e5 into development Jun 16, 2023
@muzigao-gro muzigao-gro deleted the GAIA-21588-df branch June 16, 2023 18:00
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 this pull request may close these issues.

8 participants