Skip to content

ENH: to_csv and read_{fwf/csv/table} type improvements #243

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

Closed
wants to merge 4 commits into from

Conversation

bashtage
Copy link
Contributor

@bashtage bashtage commented Sep 1, 2022

  • Tests added: Please use assert_type() to assert the type of any return value

@bashtage
Copy link
Contributor Author

bashtage commented Sep 1, 2022

@Dr-Irv Looks like the loguru stuff fixed itself?

@bashtage
Copy link
Contributor Author

bashtage commented Sep 1, 2022

Maybe not fixed, but random. Might depend on the CI worker node.

@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Sep 1, 2022

Maybe not fixed, but random. Might depend on the CI worker node.

Yes, this randomness is part of the problem. I don't know what to do about it.

Copy link
Collaborator

@Dr-Irv Dr-Irv left a comment

Choose a reason for hiding this comment

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

My only concern here is that I can't tell if you modified read_csv() or read_table() because they moved from one file to the other.

If you did change read_csv() or read_table(), can you do one PR that just modifies their signatures, but keeping them in the same file, so it is easy for me to compare what you changed, and then a second PR that moves them from where they are in main now to the new place you put them.

@bashtage bashtage closed this Sep 1, 2022
@bashtage
Copy link
Contributor Author

bashtage commented Sep 1, 2022

If you did change read_csv() or read_table(), can you do one PR that just modifies their signatures, but keeping them in the same file, so it is easy for me to compare what you changed, and then a second PR that moves them from where they are in main now to the new place you put them.

Created #248 in place of this

@bashtage bashtage deleted the io-fwf branch September 3, 2022 23:27
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.

2 participants