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

Allow submitting coords/data as a tibble #128

Closed
wants to merge 1 commit into from

Conversation

Moohan
Copy link

@Moohan Moohan commented May 2, 2024

This change will not change or break any existing behaviour but will work if the coordinates are submitted in a tibble.

I was trying to submit coordinates in a tibble and was getting an odd error which I traced to the utils script. Since tibble inherits from data.frame it was passing the first check but was then causing an error later since x[,1] on a tibble will return a single column tibble rather than the expected vector.

All the tests pass but I didn't add any new ones as I couldn't see a way to do that without at least adding a development dependency on {tibble}

This change will not change or break any existing behaviour but will work if the coordinates are submitted in a tibble.
@rCarto
Copy link
Member

rCarto commented May 2, 2024

Hello,
Thank you very much for raising this issue. It would be nice to accept tibbles in the pkg.
I think I'll rather go with a solution that makes data.frames defaults explicit.

rCarto added a commit that referenced this pull request May 2, 2024
@rCarto
Copy link
Member

rCarto commented May 2, 2024

I think this should be enough to allow the use of tibbles. Thank you again for taking the time to prepare a PR and for pointing me in the right direction. Don't hesitate to reopen the PR if the fix does not work as intended.

@rCarto rCarto closed this May 2, 2024
@Moohan
Copy link
Author

Moohan commented May 2, 2024

Great, maybe I should have just opened an issue 😆 your change works for me!

@Moohan Moohan deleted the work_with_tibbles branch September 30, 2024 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants