5245 f-strings #5246

GeorgeA92 · 2021-09-23T14:16:31Z

Related to #5245
Replaced string format by usage if f-strings in places mentioned in recent pylint checks.

(edit) fixes #5245

…trings

…rings

GeorgeA92 · 2021-09-23T14:43:01Z

scrapy/extensions/httpcache.py

-        logger.debug("Using DBM cache storage in %(cachepath)s" % {'cachepath': dbpath}, extra={'spider': spider})
+        logger.debug(f"Using DBM cache storage in {dbpath}", extra={'spider': spider})


Old message from pylint related to this code line:
scrapy/extensions/httpcache.py:229:21: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)
New message from pylint related to this code line:
scrapy/extensions/httpcache.py:229:8: W1203: Use lazy % or .format() or % formatting in logging functions (logging-fstring-interpolation)

Laerte · 2021-09-23T16:57:35Z

scrapy/downloadermiddlewares/httpcache.py

@@ -70,7 +70,7 @@ def process_request(self, request: Request, spider: Spider) -> Optional[Response
            self.stats.inc_value('httpcache/miss', spider=spider)
            if self.ignore_missing:
                self.stats.inc_value('httpcache/ignore', spider=spider)
-                raise IgnoreRequest("Ignored request not in cache: %s" % request)
+                raise IgnoreRequest(f"Ignored request not in cache: {str(request)}")


Suggested change

raise IgnoreRequest(f"Ignored request not in cache: {str(request)}")

raise IgnoreRequest(f"Ignored request not in cache: {request}")

Use str inside a f-string is redudant, same for others places.

Ok, thanks @Laerte

@GeorgeA92 No problem, great work btw.

I think this wasn't implemented?

codecov · 2021-09-23T20:32:00Z

Codecov Report

Merging #5246 (a4cc04c) into master (8284de5) will increase coverage by 0.02%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master    #5246      +/-   ##
==========================================
+ Coverage   88.51%   88.53%   +0.02%     
==========================================
  Files         163      163              
  Lines       10592    10611      +19     
  Branches     1524     1559      +35     
==========================================
+ Hits         9375     9394      +19     
+ Misses        943      942       -1     
- Partials      274      275       +1

Impacted Files	Coverage Δ
scrapy/__init__.py	`84.21% <0.00%> (ø)`
scrapy/core/downloader/contextfactory.py	`87.03% <0.00%> (ø)`
scrapy/core/downloader/tls.py	`97.14% <ø> (ø)`
scrapy/utils/misc.py	`97.72% <ø> (ø)`
scrapy/extensions/feedexport.py	`95.08% <75.00%> (-0.02%)`	⬇️
scrapy/downloadermiddlewares/httpcache.py	`93.97% <100.00%> (ø)`
scrapy/extensions/httpcache.py	`95.43% <100.00%> (ø)`
scrapy/commands/check.py	`71.01% <0.00%> (ø)`
scrapy/downloadermiddlewares/httpauth.py	`100.00% <0.00%> (ø)`

wRAR · 2021-09-24T06:31:14Z

Looks like there is now a TypeError in the spider ran in tests/test_crawl.py::CrawlSpiderTestCase::test_async_def_asyncio_parse_reqs_list

GeorgeA92 · 2021-09-24T07:59:20Z

@wRAR remaining errors fixed. checks/tests passed

scrapy/core/downloader/contextfactory.py

scrapy/extensions/feedexport.py

scrapy/core/downloader/contextfactory.py

scrapy/core/downloader/tls.py

scrapy/extensions/feedexport.py

scrapy/utils/misc.py

tests/test_http_request.py

…ated

Gallaecio

I believe the only thing missing now is #5246 (comment)

tests/test_http_request.py

elacuesta · 2021-09-29T20:39:14Z

scrapy/extensions/feedexport.py

+            "Stored %s", logmsg,
+            extra={'spider': spider}


(Nitpick) I would suggest either putting everything in the same line (which should be ok given our current line length), or each argument in its own line.

GeorgeA92 · 2021-10-01T20:53:12Z

@Gallaecio
On last check received failed tests (pypy) on CallbackKeywordArgumentsTestCase.test_callback_kwargs.
Probably related to recently (14 hours ago) megred #5251

Gallaecio · 2021-10-02T06:26:41Z

Yeap, @elacuesta was aware and left the work to you. Bad @elacuesta 🙂

Gallaecio · 2021-10-02T06:28:14Z

@GeorgeA92 Also, please, do not forget to address #5246 (comment) , I think you forgot about that at some point.

Gallaecio · 2021-10-02T11:30:22Z

Yeap, @elacuesta was aware and left the work to you. Bad @elacuesta 🙂

Actually, I think I misinterpreted his comment.

GeorgeA92 added 12 commits September 23, 2021 12:13

extentions.httpcache : string format replaced by f-strings

82c26ab

downloadermiddlewares.httpcache : string format replaced by f-strings

5c622ae

scrapy.utils.misc : string format replaced by f-strings

5befc88

scrapy.core.downloader.contextfactory : string format replaced by f-s…

5372d95

…trings

scrapy.core.downloader.tls : string format replaced by f-strings

42da1f4

tests.test_closespider : string format replaced by f-strings

9b5354d

tests.test_http_request : string format replaced by f-strings

cb93322

tests.test_commands : string format replaced by f-strings

1485f61

tests.test_downloader_handlers_http2 : string format replaced by f-st…

abeb3c2

…rings

tests.spiders : string format replaced by f-strings

79c1713

scrapy.extensions.feedexport : string format replaced by f-strings

06aaf0a

scrapy.extensions.feedexport : _close_slot f-string usage optimized

8b312cb

GeorgeA92 changed the title ~~5242 f-strings~~ 5245 f-strings Sep 23, 2021

GeorgeA92 commented Sep 23, 2021

View reviewed changes

Laerte reviewed Sep 23, 2021

View reviewed changes

GeorgeA92 added 2 commits September 23, 2021 23:20

scrapy.extensions.feedexport : error fixed

9ac5b6c

tests.test_http_request : regular string used

7362fc7

GeorgeA92 added 5 commits September 23, 2021 23:35

extentions.httpcache : logger debug message formatting updated

c30db21

scrapy.__init__ : string format replaced by f-strings

dc19c7d

scrapy.core.downloader.tls : logger warning message formatting updated

ead32c1

scrapy.extensions.feedexport: logger message formatting updated

16fddcb

scrapy.extensions.feedexport: logger message formatting fixed

7f2990e

GeorgeA92 added 2 commits September 24, 2021 10:06

tests.spiders: logger message formatting fixed

5c6db3c

tests.spiders: log message fixed

799cd84

wRAR reviewed Sep 24, 2021

View reviewed changes

scrapy/core/downloader/contextfactory.py Outdated Show resolved Hide resolved

wRAR reviewed Sep 24, 2021

View reviewed changes

scrapy/extensions/feedexport.py Outdated Show resolved Hide resolved

scrapy.extensions.feedexport: logfmt -> renamed to logmsg

91b776e

scrapy.core.downloader.contextfactory : remove tuple from f-string

2a1faec

Gallaecio reviewed Sep 25, 2021

View reviewed changes

Gallaecio mentioned this pull request Sep 25, 2021

fix: Update to f-string #5247

Closed

GeorgeA92 added 5 commits September 27, 2021 13:11

code.downloader.contextfactory: warning message formatting updated

9c6b2b9

core.downloader.tls: logger message formatting updated

3ce0fad

extensions.feedexport: logmsg formatting updated

0f27f1b

utils.misc: error message formatting updated

bb62afd

tests.test_http_request: test_from_response_valid_form_methods -> upd…

67ce305

…ated

Gallaecio reviewed Sep 28, 2021

View reviewed changes

tests/test_http_request.py Outdated Show resolved Hide resolved

elacuesta reviewed Sep 29, 2021

View reviewed changes

elacuesta mentioned this pull request Oct 1, 2021

Allow 'callback' key in keyword arguments for request callbacks #5251

Merged

GeorgeA92 added 3 commits October 1, 2021 18:40

tests.test_http_request: statement formatting updated

548906e

tests.test_downloadermiddleware_stats: statement formatting updated

bec8b1c

tests.test_http_request: typo fixed

d3be17b

downloadermiddlewares.httpcache: {str(request)} -> {request}

a4cc04c

Gallaecio approved these changes Oct 2, 2021

View reviewed changes

Laerte approved these changes Oct 3, 2021

View reviewed changes

Gallaecio closed this Oct 7, 2021

Gallaecio reopened this Oct 7, 2021

Gallaecio merged commit d3f1bf7 into scrapy:master Oct 7, 2021

Gallaecio mentioned this pull request Oct 9, 2021

Updated workflow to support python 3.10 scrapy/parsel#230

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

5245 f-strings #5246

5245 f-strings #5246

GeorgeA92 commented Sep 23, 2021 •

edited by elacuesta

Loading

GeorgeA92 Sep 23, 2021

Laerte Sep 23, 2021

GeorgeA92 Sep 23, 2021

Laerte Sep 23, 2021

wRAR Sep 24, 2021

codecov bot commented Sep 23, 2021 •

edited

Loading

wRAR commented Sep 24, 2021

GeorgeA92 commented Sep 24, 2021

Gallaecio left a comment

elacuesta Sep 29, 2021

GeorgeA92 commented Oct 1, 2021 •

edited

Loading

Gallaecio commented Oct 2, 2021 •

edited

Loading

Gallaecio commented Oct 2, 2021

Gallaecio commented Oct 2, 2021 •

edited

Loading

		logger.debug("Using DBM cache storage in %(cachepath)s" % {'cachepath': dbpath}, extra={'spider': spider})
		logger.debug(f"Using DBM cache storage in {dbpath}", extra={'spider': spider})

	raise IgnoreRequest(f"Ignored request not in cache: {str(request)}")
	raise IgnoreRequest(f"Ignored request not in cache: {request}")

5245 f-strings #5246

5245 f-strings #5246

Conversation

GeorgeA92 commented Sep 23, 2021 • edited by elacuesta Loading

GeorgeA92 Sep 23, 2021

Choose a reason for hiding this comment

Laerte Sep 23, 2021

Choose a reason for hiding this comment

GeorgeA92 Sep 23, 2021

Choose a reason for hiding this comment

Laerte Sep 23, 2021

Choose a reason for hiding this comment

wRAR Sep 24, 2021

Choose a reason for hiding this comment

codecov bot commented Sep 23, 2021 • edited Loading

Codecov Report

wRAR commented Sep 24, 2021

GeorgeA92 commented Sep 24, 2021

Gallaecio left a comment

Choose a reason for hiding this comment

elacuesta Sep 29, 2021

Choose a reason for hiding this comment

GeorgeA92 commented Oct 1, 2021 • edited Loading

Gallaecio commented Oct 2, 2021 • edited Loading

Gallaecio commented Oct 2, 2021

Gallaecio commented Oct 2, 2021 • edited Loading

GeorgeA92 commented Sep 23, 2021 •

edited by elacuesta

Loading

codecov bot commented Sep 23, 2021 •

edited

Loading

GeorgeA92 commented Oct 1, 2021 •

edited

Loading

Gallaecio commented Oct 2, 2021 •

edited

Loading

Gallaecio commented Oct 2, 2021 •

edited

Loading