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

fill, docstring, istypes, benchmark #30

Merged
merged 19 commits into from
Aug 22, 2023

Conversation

drizk1
Copy link
Member

@drizk1 drizk1 commented Aug 18, 2023

  • I was unable to get parse_across to support the predicate functions, so the is_type do not have much functionality right now but included are is_float, is_integer, is_string.

  • fill_missing macro + docstring + example

  • slice_sample macro + docstring + addended slice in user guide

  • added a benchmarking file to the docs. it may totally miss the mark with delivery. I am happy to revise as needed.

@drizk1
Copy link
Member Author

drizk1 commented Aug 18, 2023

draft.

@drizk1
Copy link
Member Author

drizk1 commented Aug 18, 2023

Ready for review.

jk not yet.

now its ready. removed is_categorical because I will just throw that in TidierCats to minimize adding new deps.

@kdpsingh
Copy link
Member

Thank you for working on this! Will take a look this weekend. My VSCode is acting up right now.

@kdpsingh
Copy link
Member

kdpsingh commented Aug 20, 2023

Nice job on fixing the doctests! I'll got through these next week. There's a lot in this PR, so I want to make sure we get each new macro/function right.

I agree with moving is_categorical() into TidierCats.

@drizk1
Copy link
Member Author

drizk1 commented Aug 20, 2023

Sounds good. Only thing I couldn't figure out was the doc test output for slice_sample because it shows a random set of rows. I tried setting a seed but that did not work either so I suppressed output w ;
I also didn't know if there was another way I should do it beside using randperm

@kdpsingh
Copy link
Member

I have ideas on how to fix.

@kdpsingh
Copy link
Member

No worries about the failing tests, has to do with dependencies. Will come back to this.

@kdpsingh kdpsingh merged commit 56c4142 into TidierOrg:main Aug 22, 2023
3 checks passed
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