Much simpler locale detection, now relying on Qt's QLocale.

[tmontes]

After all the code and efforts around #672, I came to realise that Qt can handle that for us. Thus, this PR throws all of it away, simplifies/adjusts a few tests around getdefaultlocale calls which are no longer used, replacing it all with Qt's QLocale calls as I suggested in #763.

Pros:

• Less code to maintain.
• Qt respects the LANG environment variable across platforms thus, running something like LANG=es_ES <path-to-mu-executable> now works on macOS allowing for quick testing of alternate locales regardless of the System Preferences (and I have a hunch that the same will be valid for Windows too); that was already the case in Linux so no changes there.

Cons:

• Here we go changing something that is sensitive UI-wise.

IMPORANT

• I've tested this on macOS with full success: command line, Finder, various languages.
• (post-EDIT) Tested on Ubuntu 18.04, command line only: works nicely and respects the LANG environment variable.
• (post-EDIT) Tested on Windows 10, command line only: works nicely and respects the LANG environment variable (cool!).
• It should be human-tested on the other platforms before merging, though. We don't want regressions that automated tests would have trouble catching! :)

Feedback welcome.

[tmontes]

Much like in my other recent PR #741, Travis and AppVeyour checks failed:

• Travis is complaining about over indentation in a file this PR hasn't touched.
  See the explanation in my comment there.

• AppVeyor didn't like an assertion, again, in a file this PR hasn't touched.
  See my notes in my comment there and also here

These two minor things need to be adjusted outside of this PR's scope before merging, right? :)

[tmontes]

Interesting, AppVeyor timed out while trying to discover tests. I guess I'll have to make a "dummy push" to force checks to be rerun and, hopefully, avoid timeouts this time... :)

[tmontes]

Something fishy is going on here, AppVeyor timed out again.

Went setting up a Windows dev environment and here's what I found:

• The current PR leads to a failure while running pip install -e . in a fresh virtual environment (I understand the dev setup instructions say pip install -r requirements.txt, but I went the other way and, luckily, hit this). Motive: mu/__init__.py, which now imports Qt, is imported from setup.py for the purposes of version collection.
• This should to be addressed, but does not seem to be the motive for the AppVeyor timeout.
• Having worked around that (notes on this below), I then ran make check which successfully:
  • Cleaned
  • Ran pyflakes
  • Ran pep8
  • Found 757 tests.
  • Passed all tests with a single warning (the same I currently get on macOS, wrt .../qtconsole/jupyter_widget.py:531).
• Unexpected results:
  • During the tests a browser is opened with the homepath/foo.qwerty URL.
  • Line 432 in mu/modes/microbit.py is not covered (Thread sync issues?).
  • These were observed both in this branch and in the current master, though.

With this, I have no idea why apparently, pytest execution seems to hang on AppVeyor.

Side note about setup.py and/vs requirements.txt

Probably deserving its own issue, for discussion. :-)

• Going pip install -r requirements.txt feels very risky given that versions are not locked there, but are locked in setup.py which, after pip install -e ., downgrades the current PyQt 5.12.1 to 5.11.3, as well as other dependencies.
• More: pip install -r requirements.txt on Windows systems fails on systems like mine with no C compiler, because scrapy depends on Twisted, for which there are no Windows binary wheels.
• Maybe we could completely forego the requirements.txt and use a setup.py with version locked install_requires for the core dependencies, and non-version locked extras_requires for tests, docs, etc. where things like pytest, sphinx, and scrapy would properly go.
• That would allow a basic pip install -e . to work, installing the minimal dependencies that are required for Mu to run, and variations like pip install -e .[dev|docs|all|...] to bring in additional development oriented dependencies.
• At first glance, such simplification (and, importantly, single source of truth) will not complicate the current Windows and macOS packaging procedures.

[tmontes]

AppVeyor did indeed time out again, much like I suspected. Same symptom: pytest seems to hang.

Facts:

• Both make test and make check complete successfully on my Windows dev system.
• Both behave similarly with current master and this PR's code.

Thoughts:

• Maybe undo 50d5596 and leave the setup.py vs. requirements.txt topic for an in-context discussion, issue, and PR.
• Could it be that the code is now calling Qt too early (whatever that means) and that changes things somehow? Hmmm...

[tmontes]

Status:

• Tried a few educated guesses at understanding / fixing the apparent AppVeyor build hang:
  • Not importing Mu in setup.py.
  • Having the code call Qt's locale detection + gettext setup later, away from mu/__init__.py.
  • Setting the LANG environment variable in AppVeyor build.
• None produced changes that I could detect.
• The underlying motive is still something I don't understand.
• Will revert those commits.

Fact:

• make test and make check both run successfully on my local dev system.

Things I find intriguing (pointers to the underlying failure?):

• Why doesn't AppVeyor output the same thing I get on my cmd.exe running the make win64 step:
  • I get a few lines saying Check, Clean, pyflakes, PEP8, and coverage before the ========= test session starts ========== line.
  • AppVeyor's log seems to skip those initial lines, displaying the ========= test session starts ========== right after the make win64 one.
• Why is a browser window being opened during the tests? (haven't investigated)

Next:

• How to determine what's really going on with the AppVeyor build? (like I said above, I feel somewhat uncomfortable with the fact that the output I get locally is different from what I'm seeing in the log)
• Try to push a full revert commit to really confirm that the Qt's locale detection is actually driving the apparent hang?
• Reproduce AppVeyor's build, step by step, version by version on my local dev system?

[tmontes]

Hmmm, interesting:

• The AppVeyor build now hanged during one of the tests in tests/test_logic.py.
• Hypothesis: maybe on test_show_help that still mocks PyQt5.Qt.QLocale: I forgot to remove that in my "AppVeyor diagnostic" test commit eb58cb8.

Will clean that up and try again.

[tmontes]

Confirmed:

• Using Qt's QLocale leads to AppVeyor hangs which seem to happen during test discovery.
• Mocking Qt's QLocale leads to AppVeyor hangs which seem to happen during test execution.

Thought:

• Could this be related to the fact that AppVeyor's systems have multiple Qt versions installed?
• Could we be facing a "wrong DLL version" being loaded at some point?
• No idea! :)

Moving forward:

• Revert the last two "AppVeyor diagnostic" commits to leave the PR in a "decent albeit not mergeable" state.
• Feeling the urge to create a minimal diagnostic project:
  • Minimal amount of code.
  • Using and Mocking Qt's QLocale in pytest driven tests.
  • AppVeyor tested.
  • Not clogging up Mu's AppVeyor build queue. :)

[tmontes]

For transparency and completeness:

• I created this minimal test project to help me understand what is failing with this PR and AppVeyor.
• Currently pushing to master.
• AppVeyor page is here.

Interestingly it seems to be leading to AppVeyor hangs in the same way. Good. :)

[ntoll]

@tmontes wow... you've had a "fun" Sunday..! :-)

Thank you for all the effort you're putting into this. As someone who spent a couple of days last week messing around with broken Travis CI builds... I feel your pain with Appveyor.

[tmontes]

@ntoll, I confess this has been bugging me a lot. The lack of AppVeyor output drives me crazy! :)

[tmontes]

After some exploration (and frustration) in a minimal code sandbox here, I finally did it:

• The fix is commit 6e93d2b.
• The curlprit was the way QLocale was being imported: for some reason from PyQt5.Qt import QLocale works locally but hangs AppVeyor imports. Using from PyQt5.QtCore import QLocale instead, makes it all work.
• Why on earth that is the way it is I have no idea, being no PyQt expert: could this be flagged as PyQt bug? AppVeyor limitation? Something else?
• More: why can QLocale be imported from different modules? (there's probably a good reason for that, any pointers?)

[ntoll]

Thank you @tmontes for this difficult but welcome change. You deserve a gold medal for fighting with the "AppVeyor" beast. :-)

[carlosperate]

Wow, great work Tiago! That was quite a journey to tame the beast 🐉 .

One question I have, which you have raised as well: Isn't having a Qt import in __init__.py going to cause issues installing the mu package?

From #764 (comment):

• The current PR leads to a failure while running pip install -e . in a fresh virtual environment (I understand the dev setup instructions say pip install -r requirements.txt, but I went the other way and, luckily, hit this). Motive: mu/__init__.py, which now imports Qt, is imported from setup.py for the purposes of version collection.

@ntoll Can the code running from importing mu (in __init__.py) be added to the top of run() in app.py? Would that break anything?

mu/mu/__init__.py

Lines 6 to 13 in a76a68c

	# Configure locale and language
	# Define where the translation assets are to be found.
	localedir = os.path.abspath(os.path.join(os.path.dirname(__file__), 'locale'))
	language_code = QLocale.system().name()
	# DEBUG/TRANSLATE: override the language code here (e.g. to Chinese).
	# language_code = 'zh'
	gettext.translation('mu', localedir=localedir,
	languages=[language_code], fallback=True).install()

[ntoll]

@carlosperate hehehe... you beat me to it. One of the upcoming changes I want to make is to address the several requests for manually setting (and saving) the locale that Mu uses. This will take the form of a new "UI" tab in the admin dialog which will contain such a setting (among other things). Ergo, the place where the locale is set will need to be moved anyway for this to work... hence my decision to merge now and fix pre-Alpha 1 IYSWIM.

Does this make sense? Happy to explain further if not. Basically, I'd like to merge things even if the work on them needs to be "finished" in a sense before we do alpha 1.

[ntoll]

More context... the locale should be set on startup after the settings have been loaded. If there's no user defined setting, then Mu should try to guess from the OS as we've done so before a la this PR. One of the options in the UI will be "Just use the OS's locale, if possible".

[carlosperate]

Sounds great! 👍

I guess master will have this pip install . issue momentarily until that feature is implemented, but at the moment this should only affect developers and it'll be pretty easy for most people to notice that they can do pip install -r requirements.txt && pip install -e . to workaround it.

[tmontes]

A few post merger thoughts, along with feedback to the topics @carlosperate and @ntoll raised:

• Nicholas, thanks for the medal. I'll wear it proudly. :)
• Carlos, regarding importing Qt in mu/__init__.py: that's a limitation I highlighted, yes. I'd argue that the issue stands in setup.py importing mu, which it shouldn't. Note how I quickly hacked around that in 50d5596, inspired by a cleaner approach I used here, mostly copied from Hynek's Sharing Your Labor of Love: PyPI Quick and Dirty article.
  Like I mentioned, I feel that Mu developer's lives could be much simplified by the ideas I shared in the comment you quoted, dropping requirements.txt, and improving setup.py.
• Regarding moving the mu/__init__.py code somewhere else: there's a non-obvious challenge ahead (which I barely explored in this PR's journey). Given the way most of the code is laid out, the gettext.translation call in that block must be executed very early on. Motive: it installs a _() builtin-like function that is used in places such as mu/logic.py's global MOTD. In other words, with the existing code layout, that call must be completed before importing mu/logic.py, for example (and the same applies to all mu/modes/api package modules!).
• Nicholas, regarding user selectable UI language, have you considered my proposal in Allow users to override locale setting #692, where I explain why I thing such UI option should not be "buried" in the Administration dialog?

PS: I gather that further discussion around these topics can continue under their own issues, do you agree? :)

[ntoll]

@tmontes as always, thank you for your thoughtful and welcome contributions.

I agree we should discuss these topics in their own issues and I'll try to answer / address and progress the discussions on those tickets later today.

As a result, I think we should have a new issue for the requirements.txt / setup.py discussion. See #776 for this!