Page MenuHomePhabricator

Usage of unicode_literals from __future__ package
Closed, ResolvedPublic

Subscribers
Tokens
"Party Time" token, awarded by D3r1ck01."Like" token, awarded by Rubin16."Mountain of Wealth" token, awarded by Ladsgroup."Mountain of Wealth" token, awarded by Ricordisamoa.
Assigned To
Authored By
XZise, Feb 15 2015

Description

Hi, the code should use unicode_literals throughout to avoid using u'…' which is one the reasons why Python 3 versions before 3.3 aren't supported. @jayvdb said somewhere (determine), that he'd prefer to do that in one go. I suggest to atleast do it in all new code, so that changing u'…' into '…' won't break git blame for those files when it's introduced later. It is possible to use u'…' on conjunction with unicode_literals, but that could confuse newcomers into thinking that u'…' is necessary.

One primary advantage is that the usage of ur'…' has been disallowed in Python 3 so unicode_literals is a way around it, as that does allow to use r'…' with unicode chars inside.

In case we want to revert the unicode_literals patch 1e54a7d6 the following patches are solely fixing the original patch and could be reverted with that too:

Details

Related Gerrit Patches:

Related Objects

Event Timeline

XZise created this task.Feb 15 2015, 11:56 AM
XZise raised the priority of this task from to Low.
XZise updated the task description. (Show Details)
XZise added subscribers: XZise, jayvdb.
Restricted Application added subscribers: Aklapper, Unknown Object (MLST). · View Herald TranscriptFeb 15 2015, 11:56 AM

I think it makes sense to add unicode_literals everywhere in a big bang before 2.0 RC1, and wait for the screams. This was discussed a little on https://gerrit.wikimedia.org/r/#/c/189914/2/scripts/cosmetic_changes.py

We can then remove u'...' over time, as part of regular code modifications.
I dont see that removing them particularly helps newcomers; writing a style guide for new developers is a better approach.

XZise updated the task description. (Show Details)Feb 15 2015, 12:20 PM
XZise set Security to None.

Regarding changing u'…': Obviously that doesn't need to happen independently. But (especially after unicode_literals have been imported) it's possible to change it when the line is changed anyway.

Ricordisamoa added a subscriber: Ricordisamoa.
gerritbot added a subscriber: gerritbot.

Change 186962 had a related patch set uploaded (by XZise):
[IMPROV] Use unicode_literals in fixes

https://gerrit.wikimedia.org/r/186962

Patch-For-Review

Change 202373 had a related patch set uploaded (by XZise):
[IMPROV] Use unicode_literals

https://gerrit.wikimedia.org/r/202373

Change 202373 merged by jenkins-bot:
[IMPROV] Use unicode_literals

https://gerrit.wikimedia.org/r/202373

Hmm... it seems to be causing even more harm than benefits, doesn't it?

XZise added a comment.Apr 12 2015, 9:24 AM

Well it discovered some flaws and afaik all patches after that fix the original patch work independently. So if it's really unusable we can still just revert that one patch.

XZise added a comment.Apr 12 2015, 9:38 AM

But just for good measure I'll list all patches which are unicode_literals specific in the first post so that if we want to abandon that project we could revert everything of that (but leave the useful stuff).

XZise updated the task description. (Show Details)Apr 12 2015, 9:44 AM
XZise updated the task description. (Show Details)Apr 12 2015, 11:29 AM
Xqt added a subscriber: Xqt.Jun 23 2015, 5:28 AM
XZise changed the status of subtask T114487: Pywikibot master setup.py won't install from Duplicate to Resolved.Oct 4 2015, 11:04 PM
Xqt added a subscriber: Dalba.May 23 2018, 5:33 AM

@Dalba: could you explain why this is open again or better what is still to do here?

Dalba added a comment.EditedMay 23 2018, 5:55 AM

This task has never been closed as far as I can tell.

There is one subtask: T95810: config does differ between unicode and str which can be investigated independently.

The only thing that seems to be remaining here is to decide about the complete removal of the u prefixed string literals. I'm personally in favor of it, but if others disagree, this task can be closed.

Xqt added a comment.Sep 23 2018, 8:32 AM

The only thing that seems to be remaining here is to decide about the complete removal of the u prefixed string literals. I'm personally in favor of it, but if others disagree, this task can be closed.

I aggree now. This mixed types are harder to read imho.

Change 462270 had a related patch set uploaded (by D3r1ck01; owner: Alangi Derick):
[pywikibot/core@master] [cleanup] cleanup scripts/welcome.py

https://gerrit.wikimedia.org/r/462270

Change 462270 merged by jenkins-bot:
[pywikibot/core@master] [cleanup] cleanup scripts/welcome.py

https://gerrit.wikimedia.org/r/462270

Dvorapa assigned this task to D3r1ck01.Sep 29 2018, 9:16 AM
Dvorapa removed a project: Patch-For-Review.
D3r1ck01 added a project: User-D3r1ck01.

Change 463604 had a related patch set uploaded (by D3r1ck01; owner: Alangi Derick):
[pywikibot/core@master] [cleanup] cleanup tests/[data_ingestion_tests.py-exceptions_tests.py]

https://gerrit.wikimedia.org/r/463604

D3r1ck01 moved this task from Backlog to Doing [WIP] on the User-D3r1ck01 board.Sep 29 2018, 7:39 PM

Change 463604 merged by jenkins-bot:
[pywikibot/core@master] [cleanup] cleanup tests/[data_ingestion_tests.py-exceptions_tests.py]

https://gerrit.wikimedia.org/r/463604

@Xqt, do we still have instances of u'' or u"" in the code base to remove? Or has this been complete?

Xqt added a comment.Oct 2 2018, 10:25 AM

@Xqt, do we still have instances of u'' or u"" in the code base to remove? Or has this been complete?

I guess not, seems it's completed

Xqt added a comment.Oct 2 2018, 10:44 AM

@Xqt, do we still have instances of u'' or u"" in the code base to remove? Or has this been complete?

There are some remaining "bad quotes": https://integration.wikimedia.org/ci/job/pywikibot-core-tox-docker/4720/console

Let me clean those up @Xqt :)

Change 463939 had a related patch set uploaded (by D3r1ck01; owner: Alangi Derick):
[pywikibot/core@master] [cleanup] cleanup some tests/ files

https://gerrit.wikimedia.org/r/463939

Change 463939 merged by jenkins-bot:
[pywikibot/core@master] [cleanup] cleanup some tests/ files

https://gerrit.wikimedia.org/r/463939

@Xqt, anything left?

Xqt added a comment.Oct 2 2018, 12:57 PM

@Xqt, anything left?

docs/conf.py is the last one I guess

Change 463954 had a related patch set uploaded (by D3r1ck01; owner: Alangi Derick):
[pywikibot/core@master] [cleanup] cleanup docs/conf.py

https://gerrit.wikimedia.org/r/463954

D3r1ck01 added a comment.EditedOct 2 2018, 1:21 PM

docs/conf.py is the last one I guess

Done! Since Pywikibot is to be added to codesearch, we could use it to also search though other repos in case we have some instances of this left? Maybe?

Change 463954 merged by jenkins-bot:
[pywikibot/core@master] [cleanup] cleanup docs/conf.py

https://gerrit.wikimedia.org/r/463954

Xqt closed this task as Resolved.Oct 2 2018, 6:55 PM