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

Change name of missingstring argument to missing_value #5

Open
kdpsingh opened this issue Apr 12, 2024 · 2 comments
Open

Change name of missingstring argument to missing_value #5

kdpsingh opened this issue Apr 12, 2024 · 2 comments

Comments

@kdpsingh
Copy link
Member

I don't think missingstring always needs to be a string for all read functions, which can make the name confusing. I would consider renaming it to missing_value. For the read functions that require a string, we can always convert it to a string in the read_* function or set a type restriction in the arguments.

It's not as easy as doing a find-replace because the CSV.jl package uses the missingstring argument.

@drizk1
Copy link
Member

drizk1 commented Apr 12, 2024

I think missing_value is the better name too.

The big difference is that for read_xlsx is that it mixes types ie missing_value = ["string", 44] verses the CSV.jl supported functions are written missing_value = ["string", "44"] but I can figure out a way to to change this.

@kdpsingh
Copy link
Member Author

I think minor differences in the supported options are okay if the underlying packages we are wrapping have slight differences. We'll just make sure to document this in the docstrings.

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

No branches or pull requests

2 participants