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

fix: proper handling of 'na' values #3747

Merged
merged 8 commits into from
May 16, 2024
Merged

fix: proper handling of 'na' values #3747

merged 8 commits into from
May 16, 2024

Conversation

YpeZ
Copy link
Contributor

@YpeZ YpeZ commented May 14, 2024

What are the main changes you did:

  • changed the way pandas handles 'na' values when creating a DataFrame from either a csv or list/dict data
  • the values 'NA', 'na', 'Na' are now processed as they are
  • actual not available values are stored as empty strings
  • float and int values are not affected
  • additionally, the handling of errors in request responses is improved by including more checks in the _validate_graphql_response method

how to test:

  • the dev script now includes an extra pet that is saved (and deleted) in a schema in the save_schema tests, named 'NA' with status numpy.nan. You can verify that the name is stored as 'NA', and the status is left empty

todo:

  • updated docs in case of new feature
  • added/updated tests
  • added/updated testplan to include a test for this fix, including ref to bug using # notation

@YpeZ YpeZ requested a review from davidruvolo51 May 14, 2024 12:00
@YpeZ YpeZ changed the title fix(emx2-pyclient) proper handling of 'na' values fix(emx2-pyclient): proper handling of 'na' values May 14, 2024
Copy link
Contributor

@davidruvolo51 davidruvolo51 left a comment

Choose a reason for hiding this comment

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

Looks good. There are a couple of linting issues flagged by pylance, but everything works as expected.

@@ -610,7 +610,7 @@ def _prep_data_or_file(file_path: str = None, data: list | pd.DataFrame = None)
if type(data) is pd.DataFrame:
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Pylance also flags this block as unreachable.

@YpeZ YpeZ changed the title fix(emx2-pyclient): proper handling of 'na' values fix: proper handling of 'na' values May 16, 2024
@YpeZ YpeZ requested a review from davidruvolo51 May 16, 2024 10:19
Copy link

sonarcloud bot commented May 16, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

Copy link
Contributor

@davidruvolo51 davidruvolo51 left a comment

Choose a reason for hiding this comment

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

Looks good!

@YpeZ YpeZ merged commit c6108d5 into master May 16, 2024
3 of 5 checks passed
@YpeZ YpeZ deleted the fix/pyclient-na-values branch May 16, 2024 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants