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 read_avro kwargs + cleanup tests #41

Merged
merged 1 commit into from
Mar 31, 2023

Conversation

marctorsoc
Copy link
Contributor

@marctorsoc marctorsoc commented Oct 15, 2022

This PR solves two issues:

  1. In a previous PR, the columns kwarg of from_records was promoted to read_avro to save RAM. However, this meant that all rows were read, ignoring nrows. Same for exclude to exclude columns. Here this is fixed
  2. Tests were flaky and not working for me locally. This was due to an unspecified timezone, which makes avro dump as UTC but then when reading it infers what it can. I hypothesize this might be working since the tests are run in UTC located machines, but I hope this will work now for any place.

Let me know your thoughts

@marctorsoc
Copy link
Contributor Author

@ynqa and @ruben-trdj for exposure

@deleted
Copy link
Contributor

deleted commented Oct 26, 2022

FWIW, I can confirm from local testing (Ubuntu 20.04) that:

  • tests on yqna:master fail with my system locale set to local time and pass when I set my locale to UTC
  • tests on this branch pass regardless of system locale.

@ynqa
Copy link
Owner

ynqa commented Mar 31, 2023

Once I thought to use https://github.com/spulec/freezegun for mocking or something, but it is also enough now. Thanks.

@ynqa ynqa merged commit a4b6ed6 into ynqa:master Mar 31, 2023
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.

3 participants