fixed DeprecationWarning #737

LarryMartell · 2023-07-24T13:08:04Z

fixed DeprecationWarning

Fixes #727

Removes support for Python 3.7.

claudep · 2023-07-24T13:10:35Z

As I commented in the previous PR, Python 3.7 is EOL, so it should be removed from the test matrix. The the install-requires will not be needed any more.

LarryMartell · 2023-07-24T15:17:35Z

So are you saying this issue can be closed and this PR is not needed any more?

claudep · 2023-07-24T15:19:21Z

No, it's not. However, if 3.7 support is dropped, this will simplify this patch, as the try/except will not be necessary.

LarryMartell · 2023-07-24T17:10:38Z

I have a PR our to fix the tests in master. Shall I remove the 3.7 support in that branch?

claudep

Thanks, you are on the right track!

.github/workflows/test.yml

setup.py

sorl/__init__.py

LarryMartell · 2023-07-24T23:43:53Z

After making these small changes the tests are failing in my forked branch, but it's not clear why.

https://github.com/LarryMartell/sorl-thumbnail/actions/runs/5650549930/job/15307090121#step:10:458

uri-rodberg · 2023-07-25T00:36:13Z

@claudep This PR looks fine to me now. Do you still require changes or is it OK with you now? And why are the tests canceled and failing? I expected the tests to pass now.

LarryMartell · 2023-07-25T03:37:55Z

The tests were passing until I made some very small changes Claude requested. It’s not clear to me at all why they’re failing now.

…

On Mon, Jul 24, 2023 at 5:36 PM אורי ***@***.***> wrote: @claudep <https://github.com/claudep> This PR looks fine to me now. Do you still require changes or is it OK with you now? And why are the tests canceled and failing? I expected the tests to pass now. — Reply to this email directly, view it on GitHub <#737 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJ7MIFNAXQL6CYCWH2NIALXR4IIRANCNFSM6AAAAAA2VR6BWQ> . You are receiving this because you authored the thread.Message ID: ***@***.***>

LarryMartell · 2023-07-25T05:20:06Z

The tests are very persnickety. The presence or absence of a blank line seems to make them pass or fail. On Mon, Jul 24, 2023 at 8:37 PM Larry Martell ***@***.***> wrote:

…

The tests were passing until I made some very small changes Claude requested. It’s not clear to me at all why they’re failing now. On Mon, Jul 24, 2023 at 5:36 PM אורי ***@***.***> wrote: > @claudep <https://github.com/claudep> This PR looks fine to me now. Do > you still require changes or is it OK with you now? And why are the tests > canceled and failing? I expected the tests to pass now. > > — > Reply to this email directly, view it on GitHub > <#737 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAJ7MIFNAXQL6CYCWH2NIALXR4IIRANCNFSM6AAAAAA2VR6BWQ> > . > You are receiving this because you authored the thread.Message ID: > ***@***.***> >

uri-rodberg · 2023-07-25T05:40:17Z

The tests are very persnickety. The presence or absence of a blank line seems to make them pass or fail. On Mon, Jul 24, 2023 at 8:37 PM Larry Martell @.***> wrote:
…
The tests were passing until I made some very small changes Claude requested. It’s not clear to me at all why they’re failing now.

@LarryMartell Can you please add a blank line at the end of .github/workflows/test.yml, and a blank line at the end of sorl/__init__.py, and then we will check if the tests pass?

claudep · 2023-07-25T07:16:12Z

I created #738 to only make the Python 3.7 drop (and let Larry as author), so we can then concentrate on the version issue only.

claudep · 2023-07-25T07:47:03Z

According to https://pypi.org/project/setuptools-scm/#retrieving-package-version-at-runtime, we should still catch PackageNotFoundError for cases where the package is not installed (typically during tests or dev).

LarryMartell · 2023-07-25T14:18:05Z

test.yml already had a blank line. Added one to __init__.py, tests still failing. ‪On Mon, Jul 24, 2023 at 10:40 PM ‫אורי‬‎ ***@***.***> wrote:‬

…

The tests are very persnickety. The presence or absence of a blank line seems to make them pass or fail. On Mon, Jul 24, 2023 at 8:37 PM Larry Martell *@*.***> wrote: … <#m_-5043465268867401432_> The tests were passing until I made some very small changes Claude requested. It’s not clear to me at all why they’re failing now. @LarryMartell <https://github.com/LarryMartell> Can you please add a blank line at the end of .github/workflows/test.yml, and a blank line at the end of sorl/__init__.py, and then we will check if the tests pass? — Reply to this email directly, view it on GitHub <#737 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJ7MICU2CHH75GCZKHO6ZTXR5L4XANCNFSM6AAAAAA2VR6BWQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

claudep · 2023-07-25T14:19:07Z

Blank lines have nothing to do with the failure. Please read my comment above.

LarryMartell · 2023-07-25T14:23:38Z

Here is the error from the test: sorl/__init__.py:8:1: W391 blank line at end of file Which comment? About PackageNotFound? I added that.

…

On Tue, Jul 25, 2023 at 7:19 AM Claude Paroz ***@***.***> wrote: Blank lines have nothing to do with the failure. Please read my comment above. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: ***@***.***>

codecov · 2023-07-25T14:26:08Z

Codecov Report

Merging #737 (ab75335) into master (012258b) will not change coverage.
The diff coverage is 100.00%.

❗ Current head ab75335 differs from pull request most recent head 60ee202. Consider uploading reports for the commit 60ee202 to get more accurate results

@@           Coverage Diff           @@
##           master     #737   +/-   ##
=======================================
  Coverage   74.11%   74.11%           
=======================================
  Files          30       30           
  Lines        1700     1700           
=======================================
  Hits         1260     1260           
  Misses        440      440

Files Changed	Coverage Δ
sorl/__init__.py	`100.00% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

claudep · 2023-07-25T14:29:43Z

Looks good. Try now to rebase on master, the only changed file should be sorl/__init__.py.

LarryMartell · 2023-07-25T14:33:39Z

Removed blank line, tests are passing now.

…

On Tue, Jul 25, 2023 at 7:30 AM Claude Paroz ***@***.***> wrote: Looks good. Try now to rebase on master, the only changed file should be sorl/__init__.py. — Reply to this email directly, view it on GitHub <#737 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJ7MIFGD3EWYDAIHFM52RLXR7J6HANCNFSM6AAAAAA2VR6BWQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

uri-rodberg · 2023-07-25T15:28:15Z

@LarryMartell Do you know how to rebase? This PR should be rebased on branch master. It looks to me it's still not rebased. Please read about rebasing and rebase all your commits on branch master.

LarryMartell · 2023-07-25T16:18:15Z

Yes, I know how to rebase. My 727.1 branch has been rebased with master.

uri-rodberg · 2023-07-25T16:44:07Z

Yes, I know how to rebase. My 727.1 branch has been rebased with master.

@LarryMartell You rebased origin/master and not upstream/master. Please run the following commands:

git remote add upstream https://github.com/jazzband/sorl-thumbnail.git

git remote -v

The results should be:

origin  https://github.com/LarryMartell/sorl-thumbnail.git (fetch)
origin  https://github.com/LarryMartell/sorl-thumbnail.git (push)
upstream        https://github.com/jazzband/sorl-thumbnail.git (fetch)
upstream        https://github.com/jazzband/sorl-thumbnail.git (push)

git fetch upstream

git rebase upstream/master

Remember that any time when we ask you to rebase branch master, we mean upstream/master and not origin/master. But it's a good practice to have origin/master always point to the same commit as upstream/master.

The results should be 2 files changed from upstream/master, one of them only with one blank line. Currently the PR shows me 5 files changed. If it doesn't change, please close this PR and create a new one, because maybe the PR doesn't take into account changes to branch master made after you created this PR.

LarryMartell · 2023-07-25T16:58:21Z

I have a fork and from github I sync my fork's master with the upstream, so it should be the same. But in any case I followed your exact directions and rebased. The 5 files shown as modified have indeed all been modified in my branch. ‪On Tue, Jul 25, 2023 at 9:44 AM ‫אורי‬‎ ***@***.***> wrote:‬

…

Yes, I know how to rebase. My 727.1 branch has been rebased with master. @LarryMartell <https://github.com/LarryMartell> You rebased origin/master and not upstream/master. Please run the following commands: git remote add upstream https://github.com/jazzband/sorl-thumbnail.git git remote -v The results should be: origin https://github.com/LarryMartell/sorl-thumbnail.git (fetch) origin https://github.com/LarryMartell/sorl-thumbnail.git (push) upstream https://github.com/jazzband/sorl-thumbnail.git (fetch) upstream https://github.com/jazzband/sorl-thumbnail.git (push) git fetch upstream git rebase upstream/master Remember that any time when we ask you to rebase branch master, we mean upstream/master and not origin/master. But it's a good practice to have origin/master always point to the same commit as upstream/master. The results should be 2 files changed from upstream/master, one of them only with one blank line. Currently the PR shows me 5 files changed. If it doesn't change, please close this PR and create a new one, because maybe the PR doesn't take into account changes to branch master made after you created this PR. — Reply to this email directly, view it on GitHub <#737 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAJ7MIB7BVXYDIR2NGJNLI3XR7ZWFANCNFSM6AAAAAA2VR6BWQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

LarryMartell · 2023-07-25T18:59:09Z

There are 2 things in this branch - fixing the deprecation warning and removing python 3.7. On Tue, Jul 25, 2023 at 9:57 AM Larry Martell ***@***.***> wrote:

…

I have a fork and from github I sync my fork's master with the upstream, so it should be the same. But in any case I followed your exact directions and rebased. The 5 files shown as modified have indeed all been modified in my branch. ‪On Tue, Jul 25, 2023 at 9:44 AM ‫אורי‬‎ ***@***.***> wrote:‬ > Yes, I know how to rebase. My 727.1 branch has been rebased with master. > > @LarryMartell <https://github.com/LarryMartell> You rebased > origin/master and not upstream/master. Please run the following commands: > > git remote add upstream https://github.com/jazzband/sorl-thumbnail.git > > git remote -v > > The results should be: > > origin https://github.com/LarryMartell/sorl-thumbnail.git (fetch) > origin https://github.com/LarryMartell/sorl-thumbnail.git (push) > upstream https://github.com/jazzband/sorl-thumbnail.git (fetch) > upstream https://github.com/jazzband/sorl-thumbnail.git (push) > > git fetch upstream > > git rebase upstream/master > > Remember that any time when we ask you to rebase branch master, we mean > upstream/master and not origin/master. But it's a good practice to have > origin/master always point to the same commit as upstream/master. > > The results should be 2 files changed from upstream/master, one of them > only with one blank line. Currently the PR shows me 5 files changed. If it > doesn't change, please close this PR and create a new one, because maybe > the PR doesn't take into account changes to branch master made after you > created this PR. > > — > Reply to this email directly, view it on GitHub > <#737 (comment)>, > or unsubscribe > <https://github.com/notifications/unsubscribe-auth/AAJ7MIB7BVXYDIR2NGJNLI3XR7ZWFANCNFSM6AAAAAA2VR6BWQ> > . > You are receiving this because you were mentioned.Message ID: > ***@***.***> >

LarryMartell · 2023-07-31T03:14:40Z

Are there any issues with this PR that are preventing it from being accepted and merged?

uri-rodberg · 2023-07-31T03:50:05Z

@LarryMartell Please contact me by email. Your mailbox is full.

uri-rodberg · 2023-08-01T00:40:34Z

I'm going to create another PR instead of this PR. Closing this PR.

LarryMartell added 4 commits July 19, 2023 12:37

dummy commit to trigger tests

d563829

dummy commit to trigger tests

5ddff34

Merge branch 'jazzband:master' into master

d15b45e

fixed DeprecationWarning

db9c440

LarryMartell added 4 commits July 24, 2023 11:20

updates for tests

1ef258e

fixed for tests

eea0466

fix for tests

ac9c617

remove py3.7

4d3f79e

claudep requested changes Jul 24, 2023

View reviewed changes

.github/workflows/test.yml Outdated Show resolved Hide resolved

setup.py Show resolved Hide resolved

sorl/__init__.py Outdated Show resolved Hide resolved

address review comments

deb5785

uri-rodberg mentioned this pull request Jul 25, 2023

Delete old thumbnails from the media/cache directory and the database #733

Closed

uri-rodberg requested a review from claudep July 25, 2023 01:01

catch PackageNotFoundError, add blank line

1bdbbac

remove blank line

ab75335

LarryMartell added 3 commits July 25, 2023 09:14

Remove support for EOL Python 3.7

a59e9c3

fixed DeprecationWarning

55e8eff

remove py3.7

3eaf2c9

LarryMartell added 2 commits July 25, 2023 09:49

fixed DeprecationWarning

c7c3d5d

remove py3.7

60ee202

JanMalte approved these changes Jul 31, 2023

View reviewed changes

uri-rodberg closed this Aug 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixed DeprecationWarning #737

fixed DeprecationWarning #737

LarryMartell commented Jul 24, 2023 •

edited by uri-rodberg

Loading

claudep commented Jul 24, 2023

LarryMartell commented Jul 24, 2023 •

edited

Loading

claudep commented Jul 24, 2023

LarryMartell commented Jul 24, 2023

claudep left a comment

LarryMartell commented Jul 24, 2023

uri-rodberg commented Jul 25, 2023

LarryMartell commented Jul 25, 2023 via email

LarryMartell commented Jul 25, 2023 via email

uri-rodberg commented Jul 25, 2023

claudep commented Jul 25, 2023

claudep commented Jul 25, 2023

LarryMartell commented Jul 25, 2023 via email

claudep commented Jul 25, 2023

LarryMartell commented Jul 25, 2023 via email

codecov bot commented Jul 25, 2023 •

edited

Loading

claudep commented Jul 25, 2023

LarryMartell commented Jul 25, 2023 via email

uri-rodberg commented Jul 25, 2023

LarryMartell commented Jul 25, 2023 via email

uri-rodberg commented Jul 25, 2023

LarryMartell commented Jul 25, 2023 via email

LarryMartell commented Jul 25, 2023 via email

LarryMartell commented Jul 31, 2023

uri-rodberg commented Jul 31, 2023

uri-rodberg commented Aug 1, 2023

fixed DeprecationWarning #737

fixed DeprecationWarning #737

Conversation

LarryMartell commented Jul 24, 2023 • edited by uri-rodberg Loading

claudep commented Jul 24, 2023

LarryMartell commented Jul 24, 2023 • edited Loading

claudep commented Jul 24, 2023

LarryMartell commented Jul 24, 2023

claudep left a comment

Choose a reason for hiding this comment

LarryMartell commented Jul 24, 2023

uri-rodberg commented Jul 25, 2023

LarryMartell commented Jul 25, 2023 via email

LarryMartell commented Jul 25, 2023 via email

uri-rodberg commented Jul 25, 2023

claudep commented Jul 25, 2023

claudep commented Jul 25, 2023

LarryMartell commented Jul 25, 2023 via email

claudep commented Jul 25, 2023

LarryMartell commented Jul 25, 2023 via email

codecov bot commented Jul 25, 2023 • edited Loading

Codecov Report

claudep commented Jul 25, 2023

LarryMartell commented Jul 25, 2023 via email

uri-rodberg commented Jul 25, 2023

LarryMartell commented Jul 25, 2023 via email

uri-rodberg commented Jul 25, 2023

LarryMartell commented Jul 25, 2023 via email

LarryMartell commented Jul 25, 2023 via email

LarryMartell commented Jul 31, 2023

uri-rodberg commented Jul 31, 2023

uri-rodberg commented Aug 1, 2023

LarryMartell commented Jul 24, 2023 •

edited by uri-rodberg

Loading

LarryMartell commented Jul 24, 2023 •

edited

Loading

codecov bot commented Jul 25, 2023 •

edited

Loading