search

hatnote / montage

📷 Photo evaluation tool for and by Wiki Loves competitions
https://commons.wikimedia.org/wiki/Commons:Montage
BSD 3-Clause "New" or "Revised" License
36 stars 11 forks source link

Avoid fail when file does not exist #175

Closed baturin closed 4 years ago

baturin commented 4 years ago

Let's start sorting out issues from https://github.com/hatnote/montage/issues/141. That simple PR avoids montage from failure when importing renamed/removed/incorrect image files.

codecov-commenter commented 4 years ago

Codecov Report

Merging #175 into master will decrease coverage by 0.15%. The diff coverage is 35.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #175      +/-   ##
==========================================
- Coverage   76.52%   76.37%   -0.16%     
==========================================
  Files          19       19              
  Lines        3766     3775       +9     
  Branches      471      474       +3     
==========================================
+ Hits         2882     2883       +1     
- Misses        643      650       +7     
- Partials      241      242       +1
Impacted Files Coverage Δ
montage/labs.py 30.30% <0.00%> (ø)
montage/admin_endpoints.py 81.99% <20.00%> (-0.79%) :arrow_down:
montage/loaders.py 73.17% <36.36%> (-1.68%) :arrow_down:
montage/rdb.py 77.90% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a921c5d...9d1bb64. Read the comment docs.

baturin commented 4 years ago

@mahmoud Yes, I think we should add some tests for situations like that (there are also several situations around that should be handled in a similar way). BTW, have you checked my previous test improvements https://github.com/hatnote/montage/pull/159? As for displaying error message, that's also pretty good question, and I think yes, we should rework error display method to make it more structured and handle many errors (as there could be many files with issues like that). I'm going to think of a way once we handle all issues on backend.

mahmoud commented 4 years ago

Cool. So, what I'm understanding is that we can roll with this for now and will improve error display in a future PR?

As for #159, somehow I missed that! Left a comment. :)

mahmoud commented 4 years ago

@baturin just checking in on this, are we OK to merge + address errors in the next?

baturin commented 4 years ago

@mahmoud Yeah, let's continue improving error reporting in future PRs, and merge that one.