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

Tiff converter returns DataArray instead of Dataset. #248

Merged
merged 1 commit into from
May 7, 2024
Merged

Conversation

huard
Copy link
Contributor

@huard huard commented May 7, 2024

Overview

This PR fixes #247

Changes:

  • Added ...

Related Issue / Discussion

Additional Information

Links to other issues or sources.

Copy link
Contributor

@Zeitsperre Zeitsperre left a comment

Choose a reason for hiding this comment

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

Won't this change break support for multi-band GeoTIFFs? Is there a way to add a test for this here?

@huard
Copy link
Contributor Author

huard commented May 7, 2024

Good point, do you have a multiband geotiff laying around ?

@huard
Copy link
Contributor Author

huard commented May 7, 2024

Found one, and what happens is that bands are just a dimension of the DataArray.

@Zeitsperre
Copy link
Contributor

Found one, and what happens is that bands are just a dimension of the DataArray.

Ah, if that's the case, I'm relatively comfortable with this change. This behaviour has been around for a year, though, so it's possible that people are using. Would it be too cautious to ask for a second reviewer on this?

@Zeitsperre Zeitsperre requested a review from cehbrecht May 7, 2024 20:05
Copy link
Contributor

@Zeitsperre Zeitsperre left a comment

Choose a reason for hiding this comment

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

Approved for me, but let's wait for @cehbrecht before moving forward.

@huard
Copy link
Contributor Author

huard commented May 7, 2024

No, it's been merged last March, see https://github.com/bird-house/birdy/pull/240/files

@Zeitsperre
Copy link
Contributor

OK, if it was introduced two months ago (0.8.5), then it's clearly a regression. No need for Carsten to weigh in.

@Zeitsperre Zeitsperre removed the request for review from cehbrecht May 7, 2024 20:10
@Zeitsperre Zeitsperre merged commit 531f97c into master May 7, 2024
7 checks passed
@Zeitsperre Zeitsperre deleted the fix-247 branch May 7, 2024 20:10
@Zeitsperre
Copy link
Contributor

@huard If you want to prepare a version release, I'll sign off on it.

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.

Tiff converter returns dataset instead of DataArray
2 participants