-
Notifications
You must be signed in to change notification settings - Fork 122
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
Add File Info Extension v1.0.0 -> v2.0.0 migration #472
Comments
I am not sure we can easily transfer those information that were describing the file to one or more raster bands. There are several cases that should be supported and thus several implementation rules must be defined. A basic rule could be to apply the file fields to all raster bands corresponding fields. If the assets of the original item does not describe any raster band, is a default raster band created? |
I was thinking that if
Yeah, this is a tricky. I was thinking that we should create a default Raster Band Object so that we don't lose the information. However, that could be misleading if an asset has more than 1 band but we create only a single Raster Band Object. It also doesn't seem possible to discover the number of Raster Band Objects that should be created without opening the file, which is not something we want to do here. What is your opinion on this @emmanuelmathot? |
That is really tricky. We should avoid add raster bands to non raster assets. I would create a default raster band so that we do not loose the information . But I would add some kind of filters to be sure that the asset is a potential raster (e.g. mime-type, file extension) |
Are there legitimate cases in File Info Extension v1.0.0 where someone would have used these fields for something besides a raster? It seems like these fields were always implicitly intended for raster files and that we just made that more explicit by splitting the extensions (I may not be aware of the full evolution of these, though, so this could be off-base). If this is true, we may just want to give the publisher the benefit of the doubt and always create the Raster Band rather than trying to filter on all of the possible MIME types and extensions. However, if there are other legitimate uses this may be our only viable option. cc @m-mohr |
I guess I could construct use cases, but I doubt someone has actually implemented file for anything other than raster. I'm not sure though how to convert to raster:bands. Is it always the case that a global e.g. file:nodata, can be converted to a "single band" raster:bands containing file:nodata? We are assuming it is one band, but that may not be true. |
Given some of the uncertainties around handling the case where no Raster Band exist it might be best not to attempt any migration in that case. In that case, the logic would be to migrate
@m-mohr @emmanuelmathot How do you feel about this approach? |
Maybe additionally: If no Raster Bands already exist, create as many as eo:bands exist and add the field to each of them. But maybe I may not even try to migrate it in JS and just handle leave implementation on v1.0.0. |
I am wondering if this is still necessary given its age? I would think that most providers have probably moved to the newer version over the last 2 years. |
Let's keep the issue open but not roadmap it. IMO dropping support for an old extension version should be avoided if possible, at least after v1 of that extension. I'm soft-banking on our (as of yet undeveloped) solution for #448 being so good that it makes issues like this trivial 🤞🏼. |
FYI, some implementations still use v1 as some removed v1 functionality can't easily be mapped to other extensions (e.g. global no data value). But that also means the migration path is not trivial. |
With the bands PR this can now indeed be solved :-) |
#1265 makes it so if you load an object with any of those removed fields you get a warning. Not sure if that is enough to close this ticket, but maybe it is enough until raster bands is fully baked. |
I think it's good enough. If folks start complaining about the migration w/ warnings, we can open a ticket to actually move things over once raster bands is baked. But that's fine as a new issue if it arises, IMO. |
Support for v2.0.0 of the File Info Extension was added in #442, but the
migrate
hook was not updated to include a migration from v1.0.0 to the new version. That hook should be updated to include the migration.Based on my understanding of the changes between versions, the following fields should be migrated to Raster Bands Object fields (dropping the
file:
prefix as well):file:bits_per_sample
file:data_type
file:nodata
file:unit
@emmanuelmathot @matthewhanson @m-mohr Feel free to correct this if I got anything wrong.
The text was updated successfully, but these errors were encountered: