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

fix corner cases of wb_dims + standardize error messages #990

Merged
merged 4 commits into from
Apr 12, 2024

Conversation

olivroy
Copy link
Collaborator

@olivroy olivroy commented Apr 12, 2024

Addresses #988 (comment)

Doing some touchups locally!

R/utils.R Show resolved Hide resolved
R/utils.R Outdated Show resolved Hide resolved
R/utils.R Outdated Show resolved Hide resolved
@olivroy olivroy changed the title fix corner cases of wb_dims fix corner cases of wb_dims + standardize error messages Apr 12, 2024
Copy link
Owner

@JanMarvin JanMarvin left a comment

Choose a reason for hiding this comment

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

Thanks! Looks like this is good to go

@olivroy
Copy link
Collaborator Author

olivroy commented Apr 12, 2024

Thanks! Hope the code is clear for you. I feel like we have come a long way since #702.
Also happy you stuck with me on this one.

I feel like you are starting to like wb_dims() (almost) as much as I do🎉 ?

@olivroy olivroy merged commit 4bfa2fd into main Apr 12, 2024
9 checks passed
@olivroy olivroy deleted the wb-dims/corner branch April 12, 2024 16:45
@olivroy olivroy mentioned this pull request Apr 12, 2024
@JanMarvin
Copy link
Owner

It’s perfectly fine! And yes, we sure have. And if I think of dims I always think about wb_dims(). Just like color and wb_color().

The only thing I’m constantly sad about is that we’re still not able to select dims as freely as I want. I like to see a system where it’s possible to have various independent cells. Like “A1:B2,A3:C3“, but unfortunately that’s not yet possible. dims_to_df() would need changes and probably all related functions as well.

But we’ll see, Rome wasn’t built in a day. Soon we’ll see the 1000th issue, PR or discussion. Wouldn’t imagine that when I started

@olivroy
Copy link
Collaborator Author

olivroy commented Apr 12, 2024

I agree, but at least we have a solid foundation, we know we can trust the results, and won't be able worried to break existing code when we extend it.

@olivroy olivroy mentioned this pull request Apr 12, 2024
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.

None yet

2 participants