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

Enhance the ugly error in constructor when no data passed #8920

Merged
merged 5 commits into from
Apr 10, 2024

Conversation

aimtsou
Copy link
Contributor

@aimtsou aimtsou commented Apr 8, 2024

This fix enhances the issue 8860.
I did not add any test since I believe it is not needed for this case since we did not add any functionality.

Copy link

welcome bot commented Apr 8, 2024

Thank you for opening this pull request! It may take us a few days to respond here, so thank you for being patient.
If you have questions, some answers may be found in our contributing guidelines.

@max-sixty
Copy link
Collaborator

Thanks @aimtsou !

(a test would be nice but probably not strictly required...)

@max-sixty max-sixty added the plan to merge Final call for comments label Apr 8, 2024
@aimtsou
Copy link
Contributor Author

aimtsou commented Apr 8, 2024

@max-sixty: Thank you for the clarification, I will take a look at your tests to see how they are written.
Are commits being squashed or I have to do it?

Also i see this check fails, "Read the Docs build failed!" but I assume it is not something on my end since I did not touch the documentation.

@max-sixty
Copy link
Collaborator

I will take a look at your tests to see how they are written.

For example

def test_error_message_on_set_supplied() -> None:

Are commits being squashed or I have to do it?

We take care of that!

Thanks @aimtsou !

xarray/core/variable.py Outdated Show resolved Hide resolved
@max-sixty
Copy link
Collaborator

Looks great! A follow-up PR with a test would be good but not strictly required. And feel free to add a changelog.

These sorts of PRs are very much appreciated, please keep them coming!

(@Illviljan & @dcherian I'm merging a bit quickly because I think it's important folks can make a quick change without having other things "hung onto the Christmas tree", and I reckon error messages are underrated by us devs #8264 )

@max-sixty max-sixty merged commit 0bc686c into pydata:main Apr 10, 2024
31 checks passed
Copy link

welcome bot commented Apr 10, 2024

Congratulations on completing your first pull request! Welcome to Xarray! We are proud of you, and hope to see you again! celebration gif

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plan to merge Final call for comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ugly error in constructor when no data passed
4 participants