Skip to content

Conversation

@jomey
Copy link
Contributor

@jomey jomey commented Nov 26, 2025

The small exercise at the bottom of the page declared two DataArrays, but only used one. The solution was still correct but confusing, so I reduced the needed variable declaration to the required one and renamed.

The small exercise at the bottom of the page declared two DataArrays, but only used one. The solution was still correct but confusing, so I reduced the needed variable declaration to the required one and renamed.
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Copy link
Contributor

@ianhi ianhi left a comment

Choose a reason for hiding this comment

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

Thanks @jomey I agree that that is a little bit confusing. Though I'm not sure this is the best solution because it's a little confusing to have the same argument for both x and y and doesn't fully showcase the power.

I left a proposed change. curious what you think of that approach instead?

@github-actions
Copy link

github-actions bot commented Nov 26, 2025

🎊 PR Preview b38cf8f has been successfully built and deployed to https://xarray-contrib-xarray-tutorial-preview-pr-348.surge.sh

🕐 Build time: 0.011s

🤖 By surge-preview

Vary the showcase x and y coords as suggested by @ianhi
@ianhi ianhi merged commit 960a5ee into xarray-contrib:main Nov 26, 2025
3 checks passed
@ianhi
Copy link
Contributor

ianhi commented Nov 26, 2025

Thanks for the contribution @jomey !

@jomey jomey deleted the patch-1 branch November 26, 2025 21:21
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