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

Update README.md #10

Merged
merged 3 commits into from
Feb 27, 2024
Merged

Update README.md #10

merged 3 commits into from
Feb 27, 2024

Conversation

rdboyes
Copy link
Member

@rdboyes rdboyes commented Feb 22, 2024

fix typos

@drizk1
Copy link
Member

drizk1 commented Feb 27, 2024

Hey @rdboyes thanks for catching these glaring typos.

Not sure why the build failed just because of the changes in that first commit.

Lmk if there's anything I can do to help.

Side note : not sure if adding as_integer in TidierCats is a move because it would create a conflict with TidierData but curious what ur thoughts are

@rdboyes
Copy link
Member Author

rdboyes commented Feb 27, 2024

I think the two can coexist since these "as_integer" functions only operate on CategoricalArrays - maybe @kdpsingh can confirm whether they will conflict or not. I just wrote them in because I needed them when doing the rewrite of TidierPlots, if they conflict or they're in the wrong place we can delete them

@kdpsingh
Copy link
Member

I agree with @rdboyes that TidierCats is the right home for any function that operates on CategoricalArrays.

Because of the way dispatch works in Julia, the name conflict doesn't matter across packages as long as they are operating on different data types.

So yes, adding as_integer() here is fine because the definition of the function limits its use to CategoricalArrays.

removes commented example to fix documenter (maybe?)
@rdboyes rdboyes merged commit ddc84b9 into main Feb 27, 2024
3 checks passed
@rdboyes rdboyes deleted the rdboyes-patch-1 branch February 27, 2024 21:18
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