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

Image info response must include all qualities beyond default #1809

Merged
merged 2 commits into from
May 9, 2019

Conversation

jpstroop
Copy link
Member

Closes #1754

@jpstroop jpstroop changed the title image info response must include all qualities beyond default Image info response must include all qualities beyond default Apr 15, 2019
@azaroth42
Copy link
Member

Meaning that info.json would always need to list bitonal ?

@jpstroop
Copy link
Member Author

I went back and forth. It's such an edge-case quality, and the fact that it's only required at level 2; I think this just simplifies things. I can add "or bitonal at compliance level 2" if you want, but my instinct was otherwise.

@jpstroop
Copy link
Member Author

Also, think about the developer experience. Having all of the qualities in one place (beyond default) is a better experience than having to check the qualities list AND the compliance level, IMO.

@azaroth42
Copy link
Member

I agree about the ergonomics, it's just a change. The inconsistency is if we leave bitonal in the compliance doc at level 2, then it's repeated ... which we don't do in other places. We could make it optional at level 2, forcing the publisher to put it in info.json

@jpstroop
Copy link
Member Author

jpstroop commented Apr 15, 2019

We could make it optional at level 2, forcing the publisher to put it in info.json

I like that. @zimeon @tomcrane @mikeapp ?

@mikeapp
Copy link
Member

mikeapp commented Apr 15, 2019

I would be OK with making it optional, but I assume then we're needing TRC review?

@tomcrane
Copy link
Contributor

I think bitonal is fine to be Optional at level 2.

As an art museum with lovely paintings, I have a level 2 image server but I really don't want bitonal available. It's just silly.

@jpstroop
Copy link
Member Author

Yes, I don't think I've ever heard a real world use case for bitonal

@tomcrane
Copy link
Contributor

One possible use case is harvesting a huge amount of text-bearing images, for OCR or machine learning purposes. Although I suspect most people would want to manage the conversion of the images into training set material themselves.

zimeon
zimeon previously approved these changes Apr 16, 2019
@jpstroop
Copy link
Member Author

One possible use case is harvesting a huge amount of text-bearing images, for OCR or machine learning purposes. Although I suspect most people would want to manage the conversion of the images into training set material themselves.

Yes, and we've actually never found bitonal to work as well as grayscale for OCR.

@jpstroop
Copy link
Member Author

jpstroop commented Apr 16, 2019

So, if we're in general agreement about making bitonal optional at all levels, can we merge before TRC approval, or does this need to sit open?

@jpstroop jpstroop changed the title Image info response must include all qualities beyond default [TRC-DNM] Image info response must include all qualities beyond default Apr 16, 2019
@azaroth42 azaroth42 added normative Ready-for-Eds Editorial changes ready for Editorial review Ready-for-TRC Normative changes ready for TRC review and removed Ready-for-Eds Editorial changes ready for Editorial review labels Apr 16, 2019
@zimeon zimeon changed the title [TRC-DNM] Image info response must include all qualities beyond default Image info response must include all qualities beyond default May 9, 2019
@zimeon zimeon merged commit 8fa109e into image-prezi-rc2 May 9, 2019
@zimeon zimeon deleted the 1754_quals_to_info branch May 9, 2019 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
normative Ready-for-TRC Normative changes ready for TRC review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify that level2 image API MUST for extra qualities should be in the info.json
5 participants