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

Use f-strings #4307

Closed
Gallaecio opened this issue Feb 6, 2020 · 21 comments · Fixed by #4324
Closed

Use f-strings #4307

Gallaecio opened this issue Feb 6, 2020 · 21 comments · Fixed by #4324
Assignees
Labels
Milestone

Comments

@Gallaecio
Copy link
Member

Gallaecio commented Feb 6, 2020

They offer a simpler syntax and speed, so once we drop Python 3.5 support I guess it makes sense to look into replacing % and format usages with f-strings where possible.

@ammarnajjar
Copy link
Contributor

ammarnajjar commented Feb 6, 2020

I'd like to work on this if that's ok

@elacuesta
Copy link
Member

elacuesta commented Feb 6, 2020

Thanks for the interest @ammarnajjar, please go ahead if you'd like.
FTR, seems like 3.5's end of life is set to 2020-09-13, according to https://devguide.python.org/#status-of-python-branches

@ammarnajjar
Copy link
Contributor

ammarnajjar commented Feb 6, 2020

@elacuesta do you mean it would be better to wait till 3.5's EOL?

@elacuesta
Copy link
Member

elacuesta commented Feb 6, 2020

Not really, a patch should not be merged before that, but the work can start before.

@ammarnajjar
Copy link
Contributor

ammarnajjar commented Feb 9, 2020

#4324 is open, but I don't think it is a good idea to start that early, because of the merge conflicts along the way, I will suspend the work there on my side.

@simulacra342870
Copy link

simulacra342870 commented Feb 15, 2020

I think I have the solution
I have just refactored this function within the scrapy/core/spidermw.py file:

def _fname(f):
return "%s.%s".format(
f.self.class.name,
f.func.name
)

to fstrings with same placeholders, eg %s for strings,

def _fname(f):
return f"%s.%s"(
f.self.class.name,
f.func.name
)

Don't hurt me I am extremely newbie, I am only trying to help and gain experience.
Are there any other files within the scrapy source code that needs to be refactored or do I have to look through all of source code to find a way to use fstrings to refactor the code.

cheers danny

@joybh98
Copy link
Contributor

joybh98 commented Feb 20, 2020

@ammarnajjar are you still working on this issue, if not I would like to work on it!

@ammarnajjar
Copy link
Contributor

ammarnajjar commented Feb 20, 2020

@joybhallaa , still 😄

@joybh98
Copy link
Contributor

joybh98 commented Feb 20, 2020

Cool!

@matthew02
Copy link

matthew02 commented Mar 3, 2020

Is it possible for more than one person to work on this issue at the same time? I'm trying to find a way to contribute to OSS, but I'm having a hard time finding an entry point. This is something I could handle, though.

@mHassan20896
Copy link

mHassan20896 commented Mar 10, 2020

Should I start working on it? I see people working on it already. I hope it would be a good contribution to the community

@joybh98
Copy link
Contributor

joybh98 commented Mar 11, 2020

@matthew02 @mHassan20896 there is a PR by @ammarnajjar which is a WIP(work in progress).

@Grelo4ka
Copy link

Grelo4ka commented Mar 24, 2020

Can I start working on it?

@Gallaecio
Copy link
Member Author

Gallaecio commented Mar 26, 2020

@Grelo4ka #4324 (comment)

@Grelo4ka
Copy link

Grelo4ka commented Mar 26, 2020

@Grelo4ka #4324 (comment)

Okay, in this case I'll try to find something else what I can work on

@Anupam-USP
Copy link

Anupam-USP commented Apr 25, 2020

Can more than one person work on same issue, as this seems something i can start work from?

@Gallaecio
Copy link
Member Author

Gallaecio commented Apr 27, 2020

@Anupam-USP Two people working on it at the same time would, in my opinion, be a waste of time. Moreover, in this case there is already a good pull request, but until we decide to drop Python 3.5 support from Scrapy, I don’t think it makes sense for anyone to work on this further.

@joybh98
Copy link
Contributor

joybh98 commented Apr 28, 2020

@Anupam-USP you can pick up other issues with the label good first issue
The maintainers and contributors are really helpful !

@HenriqueAJNB
Copy link

HenriqueAJNB commented Jun 27, 2020

I'd like to contribute with this issue. Can I work on it?

@siva432
Copy link

siva432 commented Jul 29, 2020

hello , I am interested to contribute to this issue.

@Gallaecio
Copy link
Member Author

Gallaecio commented Aug 5, 2020

@siva432 Sorry, this issue is already taken (#4324). The changes have not been merged yet simply because first we need to drop Python 3.5 support in Scrapy.

@Gallaecio Gallaecio modified the milestones: Python 3.6+, 2.4 Aug 17, 2020
ammarnajjar added a commit to ammarnajjar/scrapy that referenced this issue Aug 22, 2020
Conflicts resolved:
	scrapy/utils/conf.py
	scrapy/utils/ftp.py
	tests/test_feedexport.py
ammarnajjar added a commit to ammarnajjar/scrapy that referenced this issue Aug 22, 2020
ammarnajjar added a commit to ammarnajjar/scrapy that referenced this issue Aug 23, 2020
ammarnajjar added a commit to ammarnajjar/scrapy that referenced this issue Aug 25, 2020
ammarnajjar added a commit to ammarnajjar/scrapy that referenced this issue Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.