turning widgets into a module

[ginsburgnm]

Thanks for taking the time to contribute to this project. You are a star!

Checks

NOTES The unittests fail on my machine before and after this change, the failures appear to be the same though; they are all pertaining to the filebrowser.

I also made some changes to the code based on pylint standards

Issues fixed by this PR

Developer enhancement:

This turns the large mutli-class widget file into a directory, with files broken out for each class. The reason for this is that I believe it makes it easier to add new widgets. Example: If someone wants a widget that is similar in function to an existing widget but distinct enough to require a new widget, they could copy the other widget to a new file and everything (including imports) would already be available.

Additional Adds:

• Immediately pretty clear in this manner what classes depend of what imports.
• Easier to view linting issues per class, that can be a value add.
• Limit what is imported if someone does from asciimatics.widgets import *

Potential Drawback: You would now need to import the new widget into __init__.py to be able to do from asciimatics.widgets import ${new_widget} Though, that is only one additional line you would have to remember.

---

No new user features are added with this pull request, it is entirely for purposes of (hopefully) improving development.

Any other comments?

If you don't like the changes that's fine, it is your repo so ultimately you are the one that is doing the active maintenance. I just wanted to help out with a project that has helped me out.

[peterbrittain]

Thanks. This has been something that I've been considering but never quite got round to as the widgets idea took off. It's definitely a good thing if we can make it happen. Looking at the tests, I think there's a problem with python 2.7. Both CI systems failed to complete those successfully. Any chance you can take a look at that?

[ginsburgnm]

Is there a way to rerun appveyor? It took forever to run that python2 job, the travis-ci jobs talk about a stalled job, and the appveyor one doesn't actually tell us what failed.

I could always just do a meaningless commit and see the run, but that feels like a waste.

[peterbrittain]

Just tried running the branch locally:

====================================================================== ERROR: Failure: ImportError (No module named button)

Traceback (most recent call last): File "c:\work\venv27\lib\site-packages\nose\loader.py", line 418, in loadTestsFromName addr.filename, addr.module) File "c:\work\venv27\lib\site-packages\nose\importer.py", line 47, in importFromPath return self.importFromDir(dir_path, fqname) File "c:\work\venv27\lib\site-packages\nose\importer.py", line 94, in importFromDir mod = load_module(part_fqname, fh, filename, desc) File "C:\work\asciimatics\asciimatics\widgets__init__.py", line 3, in ImportError: No module named button

[ginsburgnm]

Oh cool.. Alright, I don't have a python2 environment available. I'll setup a virtual machine later today and give it a look.

[coveralls]

Coverage increased (+0.1%) to 97.284% when pulling 39096abf1bef90d81eac8c16d9c1d40122bfac1f on ginsburgnm:ngin_module into 807249f6c63197514216e3c59349ce672c6412b1 on peterbrittain:master.

[ginsburgnm]

@peterbrittain Checks have passed now. I do have a follow up question though.

How long do you anticipate supporting python2? I'm sure you know that it is EOL, and with pip EOL it's support next month with the 21.0 release; it's going to be harder and harder to support that.

[peterbrittain]

Yeah - that's a fair question. I've got the code to the point that it's ironed out those niggly version issues, so I'm happy to keep it until something forces my hand - e.g. broken dependencies, new feature needed, etc.

[peterbrittain]

Looks good. Just one thought... Does it make sense to for stuff that doesn't exist in isolation to be broken out in this way? I'm thinking, for example, the datepicker and associated popup.

[ginsburgnm]

RE: python2 Fair enough

---

RE: non-isolation classes I had the same thought actually. I can see it either way, and am fine making the change to pull those certain "internal" ones into their associated classes.

My only thought for doing it like that was consistency. Like _TempPopup should clearly be its own thing since multiple different popups pull from it. but _DatePickerPopup is clearly only used by DatePicker and _TimePickerPopup is clearly only used by TimePicker.

It's your repo, so I'll defer to your decision here. We can go how it is for a more "consistent" feel, or we can go with the merging of a couple of them for a more "each file does one 'thing'" kinda feel.

[peterbrittain]

Thanks. I'm happy with base classes staying separate. Let's just merge the concrete classes that don't work in isolation (e.g. the date/time popups). If you're happy to make the change, I'll merge the PR once the new code passes.

Oh - and since you're investing your own time into this, do please feel free to add yourself to credits.py! You deserve the recognition.

[ginsburgnm]

I think what I did matches what you were asking, let me know if you wanted anymore changes made!

[peterbrittain]

All looks good to me... I'll update CHANGES and check credits.py runs correctly.