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

feat: Start associating packaging shapes images in the taxonomy #7688

Merged
merged 12 commits into from
Nov 16, 2022

Conversation

teolemon
Copy link
Member

@teolemon teolemon commented Nov 11, 2022

What

  • Start associating packaging shapes images in the taxonomy
  • Sometimes we have several shapes per entry:
    • in that case I've tried to take the most representative
    • in some cases, create subentries that are currently commented out
  • I have added suggested materials per the official CITEO guidelines
    • They could be displayed on top of material suggestion lists, and possibly used for potential Robotoff questions

@github-actions github-actions bot added 📦 Packaging https://wiki.openfoodfacts.org/Category:Recycling 🧬 Taxonomies https://wiki.openfoodfacts.org/Global_taxonomies labels Nov 11, 2022
@teolemon teolemon added the 📦 Packaging shapes https://github.com/openfoodfacts/openfoodfacts-server/blob/main/taxonomies/packaging_shapes.txt label Nov 11, 2022
@teolemon
Copy link
Member Author

@aleene @stephanegigandet would like your input

@stephanegigandet
Copy link
Contributor

@aleene @stephanegigandet would like your input

It's a great idea to add images for each packaging shape.

Regarding the format of the property:

  • we should use image:xx: instead of image:en: so that the images apply to all languages
  • we should move the CITEO images to from html/images/icons/packaging-shapes/ to html/images/icons/packaging-shapes/citeo/ so that we can easily add other images

so the format should be something like:

image:xx:/images/icons/packaging-shapes/citeo/brique-2.svg

Regarding the content:

We may not need to add all Citeo icons as entries in the taxonomy. If we do add them, then we should have an English names for them. It might be useful to have all of them at some point (e.g. to recognize them, even though my understanding is that they will go away in France).

The focus at first should be on getting an image for all existing entries (or their parents, we can inherit the property), so that we can use them in the packaging edit interface and in knowledge panels etc.
e.g. We can assign the most generic looking bottle image to en:bottle.

@teolemon
Copy link
Member Author

@stephanegigandet 👍

  • fixed the xx lang code
  • moved the images to a dedicated CITEO directory
  • everything remains commented out for the time being, and we can uncomment as we feel confortable

@sonarcloud
Copy link

sonarcloud bot commented Nov 15, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link
Contributor

@stephanegigandet stephanegigandet left a comment

Choose a reason for hiding this comment

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

We can merge as-is, we can decide on the formatting, add image paths etc. after.

@teolemon teolemon marked this pull request as ready for review November 16, 2022 09:28
@teolemon teolemon requested a review from a team as a code owner November 16, 2022 09:28
@teolemon teolemon merged commit 88e22ef into main Nov 16, 2022
@teolemon teolemon deleted the packaging-shapes branch November 16, 2022 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📦 Packaging shapes https://github.com/openfoodfacts/openfoodfacts-server/blob/main/taxonomies/packaging_shapes.txt 📦 Packaging https://wiki.openfoodfacts.org/Category:Recycling 🧬 Taxonomies https://wiki.openfoodfacts.org/Global_taxonomies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants