search

osm-quality / wikibrain

Stores knowledge and data necessary to properly use links from OpenStreetMap to Wikipedia, Wikidata and Wikimedia Commons.
MIT License
4 stars 1 forks source link

Multiple brands throws exception #5

Closed KasperFranz closed 5 months ago

KasperFranz commented 5 months ago

This PR showcases the bug #4 and fixes #4

matkoniecz commented 5 months ago

what happens when one of them IS nonexisting? Maybe it would be better to skip this test entirely with multiple brands or split earlier, so report about nonexisting brand will actually report problematic one not merged list of them?

KasperFranz commented 5 months ago

what happens when one of them IS nonexisting? Maybe it would be better to skip this test entirely with multiple brands or split earlier, so report about nonexisting brand will actually report problematic one not merged list of them?

With this PR, it would report the brand as nonexisting if 1 of them are invalid - I am not sure how it would look in the report, I will create a quick test case for it to see output

KasperFranz commented 5 months ago

@matkoniecz do you have an example of P576 with P1011?

I would like to add a test case to ensure it is covered

matkoniecz commented 5 months ago

@KasperFranz https://www.wikidata.org/wiki/User:Mateusz_Konieczny/failing_testcases#Still_existing_brand should have some

(more tests are always welcome, I try to add them but more tested cases is always nice)

KasperFranz commented 5 months ago

I have changed the underlying function to return the dissolved brands instead, and it is now reporting the error on the invalid wikidata IDs.

Let me know if you want to adjust the error :)

matkoniecz commented 5 months ago

Thanks! And in a bit unlikely case of looking for more coding tasks: test suite has some failing tests where bug was found but is not yet fixed. Some may be even fixable in reasonable way.

matkoniecz commented 5 months ago

Hmmmm, I see

======================================================================
FAIL: test_that_wikidata_works_with_multiple_brands_one_invalid (test_wikimedia_link_issue_reporter.Tests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/mateusz/Documents/install_moje/OSM_software/wikibrain_py_package_published/test_wikimedia_link_issue_reporter.py", line 490, in test_that_wikidata_works_with_multiple_brands_one_invalid
    self.assertIsNone(should_be_fine)
AssertionError: <wikibrain.wikimedia_link_issue_reporter.ErrorReport object at 0x7f24aa898d90> is not None

Given that https://www.wikidata.org/wiki/Q7501155 is actually defunct, it should not be None, right? And if one would be invalid then error should be still emitted?

KasperFranz commented 5 months ago

Let me review it again - thank you for flagging