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

taxonomy: Add even more synonyms to jp_allergens_1 #9505

Closed

Conversation

Naruyoko
Copy link
Contributor

What

Extends #9502, referring to the same document.

@github-actions github-actions bot added 🧬 Taxonomies https://wiki.openfoodfacts.org/Global_taxonomies 🥜 Allergens categories 🥗 Ingredients labels Dec 11, 2023
@Naruyoko Naruyoko marked this pull request as ready for review December 11, 2023 08:02
@@ -91019,6 +91020,7 @@ es:Caballa
fi:makrillit
fr:Maquereaux
it:Sgombro
ja:サケ, 鮭, しゃけ, シャケ
Copy link
Collaborator

@benbenben2 benbenben2 Dec 12, 2023

Choose a reason for hiding this comment

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

ERROR - ja:サケ already is associated to en:salmons (categories) - ja:サケ cannot be mapped to entry en:mackerels

Suggested change
ja:サケ, 鮭, しゃけ, シャケ
ja:鮭, しゃけ, シャケ

Copy link
Collaborator

Choose a reason for hiding this comment

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

サケ appears 2 times in the same file, for different categories. This is the reason why the test fails.
After looking at wikipedia page and translation, I suggest to remove it from mackerels and keep it for salmons.
What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, I should have moved サバ etc. there instead, but probably copied the wrong parts of the line. Actually, the correct translations for mackerels were already there, but for some reason assigned to en:Mackerel fillets.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I pushed a fix.

@Naruyoko
Copy link
Contributor Author

It looks like there are still conflicts in the taxonomy. Some of them seem to be unrelated to my changes, but others are relevant here, for example ビーフ. I fixed some of the easy ones (and hopefully all the ones that I broke here), preferring to designate to the parent entries.

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (af42534) 49.03% compared to head (d37a272) 49.03%.

Additional details and impacted files
@@               Coverage Diff               @@
##           jp_allergens_1    #9505   +/-   ##
===============================================
  Coverage           49.03%   49.03%           
===============================================
  Files                  66       66           
  Lines               20434    20434           
  Branches             4905     4905           
===============================================
  Hits                10019    10019           
  Misses               9147     9147           
  Partials             1268     1268           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@benbenben2
Copy link
Collaborator

It looks like there are still conflicts in the taxonomy. Some of them seem to be unrelated to my changes, but others are relevant here, for example ビーフ. I fixed some of the easy ones (and hopefully all the ones that I broke here), preferring to designate to the parent entries.

Try to click on "update branch" in your PR. That should merge with new PR that have been merged. And hopefully remove the failed test and the POD error.

@Naruyoko
Copy link
Contributor Author

It looks like there are still conflicts in the taxonomy. Some of them seem to be unrelated to my changes, but others are relevant here, for example ビーフ. I fixed some of the easy ones (and hopefully all the ones that I broke here), preferring to designate to the parent entries.

Try to click on "update branch" in your PR. That should merge with new PR that have been merged. And hopefully remove the failed test and the POD error.

I don't see the button, which I think is because I'm trying to merge into jp_allergens_1 rather than main. Should I merge main into mine instead?

@benbenben2
Copy link
Collaborator

It looks like there are still conflicts in the taxonomy. Some of them seem to be unrelated to my changes, but others are relevant here, for example ビーフ. I fixed some of the easy ones (and hopefully all the ones that I broke here), preferring to designate to the parent entries.

Try to click on "update branch" in your PR. That should merge with new PR that have been merged. And hopefully remove the failed test and the POD error.

I don't see the button, which I think is because I'm trying to merge into jp_allergens_1 rather than main. Should I merge main into mine instead?

I just clicked on the button for the pr jp_allergens_1 to main. Let's see if it changes something. Otherwise, you can try to pr directly to main as you suggested.

@Naruyoko Naruyoko requested a review from a team as a code owner December 18, 2023 11:52
@github-actions github-actions bot added API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…) 🧪 additives Product Page 📦 Packaging https://wiki.openfoodfacts.org/Category:Recycling labels 🧴 Open Beauty Facts Our cosmetic analysis project https://world.openbeautyfacts.org 📚 Documentation Documentation issues improve the project for everyone. 🐾 Open Pet Food Facts Our pet food analysis project https://world.openpetfoodfacts.org 🧽 Data quality https://wiki.openfoodfacts.org/Quality 🧪 tests 📸 Open Products Facts Our project to increase the lifespan of objects. https://world.openproductsfacts.org labels Dec 18, 2023
@github-actions github-actions bot removed API Issues related to the Open Food Facts API. More specific labels exist & should be used (API WRITE…) 📦 Packaging https://wiki.openfoodfacts.org/Category:Recycling 🧴 Open Beauty Facts Our cosmetic analysis project https://world.openbeautyfacts.org 📚 Documentation Documentation issues improve the project for everyone. 🐾 Open Pet Food Facts Our pet food analysis project https://world.openpetfoodfacts.org 📸 Open Products Facts Our project to increase the lifespan of objects. https://world.openproductsfacts.org JavaScript 🐋 Docker https://docker-curriculum.com/ Food.pm Template::Toolkit The templating toolkit used by product opener. The starting point for HTML/JS/CSS fixes. GitHub Actions Pull requests that update Github_actions code 🏷️ Folksonomy Project The Folksonomy project allows to add custom data to product, and model non food products. Products update all products 🧪 integration tests labels Dec 18, 2023
Copy link

sonarcloud bot commented Dec 18, 2023

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

@benbenben2
Copy link
Collaborator

@Naruyoko, maybe that could be easier to make your pr to main directly instead of my branch.
At your place I would try to rollback to previous commit where there was only your changes and apply those changes in a new branch based on main branch.

Naruyoko added a commit to Naruyoko/openfoodfacts-server that referenced this pull request Dec 24, 2023
@Naruyoko
Copy link
Contributor Author

@Naruyoko, maybe that could be easier to make your pr to main directly instead of my branch. At your place I would try to rollback to previous commit where there was only your changes and apply those changes in a new branch based on main branch.

Okay. I made a new PR.

benbenben2 added a commit that referenced this pull request Dec 25, 2023
* jp_allergens_1

* rework and upd tests

* recreate tests

* Add more `jp` synonyms

* Reflect some `ja` allergen synonyms to other taxonomies

* jp to ja

* More jp to ja

* Fix ja for salmons and mackerels

See #9505 (comment)

* Fix some ja duplicates, preferring the parents

* Recreate tests

---------

Co-authored-by: benbenben2 <[email protected]>
@Naruyoko
Copy link
Contributor Author

Replaced by #9594.

@Naruyoko Naruyoko closed this Dec 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧪 additives 🥜 Allergens categories config 🧽 Data quality https://wiki.openfoodfacts.org/Quality dependencies Pull requests that update a dependency file ingredients analysis 🥗 Ingredients 📖 Knowledge Panels https://wiki.openfoodfacts.org/Knowledge_panels labels Product Page 🧬 Taxonomies https://wiki.openfoodfacts.org/Global_taxonomies 🧪 tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants