Import Cleanup for Services

[Ebag333]

Cleans up a lot of the cyclical import issues for \Services. Enables tox and travis and codecov (once Travis-CI and CodeCov is setup).

Issue List

• Can't delete items. Issue is with fittingView.py:
  populate = sFit.removeModule(self.activeFitID, fit.modules.index(module))
  Index is always failing to find a matching module, so always returns 0. (Odd part is, both module and fit.modules look fine to me....)
• Items view doesn't open for modules (but works fine for charges). Ship stats also doesn't work (same trace back). Item view for modules works fine out of the market, and ship item view works out of the fittings pane. It's only inside the fitting window that they have issues.

Traceback (most recent call last):
  File ".\gui\contextMenu.py", line 137, in handler
    menuHandler.activate(context, selection, i)
  File ".\gui\builtinContextMenus\itemStats.py", line 64, in activate
    ItemStatsDialog(stuff, fullContext)
  File ".\gui\itemStats.py", line 99, in __init__
    self.container = ItemStatsContainer(self, victim, item, itmContext)
  File ".\gui\itemStats.py", line 185, in __init__
    self.affectedby = ItemAffectedBy(self.nbContainer, stuff, item)
  File ".\gui\itemStats.py", line 905, in __init__
    self.PopulateTree()
  File ".\gui\itemStats.py", line 991, in PopulateTree
    self.buildModuleView(root)
  File ".\gui\itemStats.py", line 1166, in buildModuleView
    if self.stuff not in container:
TypeError: unhashable type: 'Module'

• Can't change modules through variations contexts menu (probably related to first issue)

So far those are the only issues I've found.

[Ebag333]

@blitzmann so this PR is currently working as is. I need to rebase it to clean up the merge conflicts, I'll do that tomorrow.

There are some cleanup of imports done in \Eos\ but it's very minimal. Majority of the stuff done in Eos is just code cleanup to get it to pass Tox.

I did have to disable a bunch of tests in Tox, we'll want to re-enable most of those once Eos is cleaned up (mostly freaking out about the imports). I also configured Tox to allow lines up to 130 chars, that seems to fit better with the Pyfa codebase than the default 123 (or whatever it is).

I did run some tests and didn't see any speed increases in how long it takes to calculate fits, so doesn't seem there'll be any end user benefit here. Just benefit for us with working introspection and Tox/Travis tests.

TL:DR; import service is no longer a thing.

[Ebag333]

Fixed all three issues with a single line change.

class Module(HandledItem, HandledCharge, ItemAttrShortcut, ChargeAttrShortcut, list):

Not sure where list got added in. I got git blamed for it but I don't recall making that change. vOv

@blitzmann seems everything is working now. You want to look at merging this in?

[blitzmann]

There are some cleanup of imports done in \Eos\ but it's very minimal. Majority of the stuff done in Eos is just code cleanup to get it to pass Tox.

I'm not concerned about passing formatting checks at the moment. Unless there is a compelling reason otherwise, please walk this PR back to where the imports are fixed - a formatting PR (which includes travis/tox/whatever) can be done separately (and in smaller chunks, plz - get with me on Slack for a better idea what I mean). As stated before, I prefer to have 1 thing done per PR, especially with low-level things like fucking with imports. Formatting changes for flavor-of-the-month CI, although cool, just mucks with my ability to assess the actual, real change. :)

Also, not sure if you're aware, but git allows you to cherry pick commits from another branch. So if you create a new branch with only import fixes, it would be pretty easy to pull all the formatting commits over to another branch.

[Ebag333]

I'm not concerned about passing formatting checks at the moment. Unless there is a compelling reason otherwise

The compelling reason is that Tox catches a LOT of missing references.

I can say with absolute certainty that there are deep references that I would have never caught without Tox and pyCharms code inspection. I know that you have a boatload of stuff to review, but this goes hand in hand with each other, fixing the imports lets code inspection work, and code inspection helps find broken stuff. I am 99% sure that there is stuff I fixed that was broken a long time, but no one knew because how do you tell when the majority of the file is lit up with inspection issues, many of them false flags?

There are still inspection issues that I think are things that are actually broken, but I'm only trying to correct the minimal amount to keep Tox happy.

Pyfa is spaghetti enough that trying to trace a simple thing between 5 functions in 5 files (each of which does one thing before passing it on) is super difficult. Tox helps because with fixed imports it can follow those and flag the one that's wrong.

Okay, so we could have turned off all of the other misc Tox checks and passed import reference stuff, right?

Well, then we still have files filled to the brim with inspection issues. Even after Tox was happy I continued to find issues because we have Fit and fit and Fit, and pyCharm lit it up because Fit was missing the referenced object. When half of the file is lit up those are difficult to find. I literally went file by file, line by line through Services to find these issues. That's easy to do when a dozen or less lines are flagged, hard to do when hundreds are.

TL;DR: Without cleaning up all the pep8 issues I would have never found a significant amount of issues.

Also, not sure if you're aware, but git allows you to cherry pick commits from another branch. So if you create a new branch with only import fixes, it would be pretty easy to pull all the formatting commits over to another branch.

How do you think that I caught this up to the dev branch? 😉

The formatting commits and import commits are too intertwined. Tox and pyCharm code inspection was too useful to helping sort this out.

[blitzmann]

Without cleaning up all the pep8 issues I would have never found a significant amount of issues.

I fail to see how having 11017,(space)# Skirmish Warfare Link - Interdiction Maneuvers I instead of 11017,(space)(space)# Skirmish Warfare Link - Interdiction Maneuvers I would have hidden issues. These are the types of formatting checks I could do without, and the vast majority of the changes seem to simply be that. Not only that, but formatting changes for the sake of formatting changes leads to increased risk of merge conflicts.

Please roll this back and include only import fixes. You can include tox config and whatever else, fine, but formatting (whitespace, line wrapping, line endings, double newline before classes / fucntions / anything else that isn't strictly code) can all go to a separate PR. I am not going to merge import changes without easily being able to see exactly what changed without wadding through

[Ebag333]

Let's Let's discuss this on Slack sometime

[blitzmann]

I briefly looked over this and am currently fine with most of it. I need to make a build of OS X and test due to that one conditional import changing - just need to make sure it doesn't crash...

Question about all these though:

+from eos.saveddata.implant import Implant as es_Implant
 +from eos.saveddata.character import Character as es_Character
 +from eos.saveddata.module import Slot as es_Slot, Module as es_Module
 +from eos.saveddata.fighter import Fighter as es_Fighter

Do we really need to define an alias for everything? I can understand defining aliases for services (to not confuse Fit object with Fit service), but most of these things are fine as is.

[blitzmann]

Why was this moved to the Module class? Methinks this should remain as a property of the collection.

[Ebag333]

probably. we reuse stuff all over the place. Eos does with saveddata and gamedata IIRC. When mass changing them it was easier to just alias most of them, so that I could guarantee that there weren't references pointing to the wrong object.

We probably SHOULD rename all the functions to actually be unique, but that's a lot more work.

[Ebag333]

For some reason I can see a comment you added @blitzmann in Slack, but not here. Probably because I'm on mobile.

If something got moved, it's because of cyclical imports. If you think it belongs somewhere else, then we'll have to merge the two functions, or create a stand alone library.

[blitzmann]

This is the comment:

#907 (diff)

Can you explain how it's a cyclic import? You know more about it than I do. I don't think it matters too much where it goes, but syntactically it makes more sense to keep it on the collection class.

I've finished the conflict merge, gonna make a package and test on all platforms best I can, then merge to dev and make a pre-release build

[Ebag333]

Can you explain how it's a cyclic import? You know more about it than I do. I don't think it matters too much where it goes, but syntactically it makes more sense to keep it on the collection class.

It's because of this one line here:
dummy = eos.types.Module.buildEmpty(mod.slot)

When we were third partying the imports through eos.types Python didn't know that Module used effectHandlerHelpers. As soon as we removed that, then we have Eos Module importing effectHandlerHelpers and effectHandlerHelpers importing Eos Module.

So. Have a few choices:

1. Merge Module and effectHandlerHelpers. (Probably not worth doing over a single line, and that introduces other problems.)
2. Just don't do whatever that is doing. (Yeah, kinda need that.)
3. Move the function over. Easy, and while it might feel a bit "out of place" the imports/usage only flows in one direction. (What I did.)
4. Move the function out to a stand alone library module, that imports nothing but is imported by lots of stuff. Probably most ideal for most cases, but since we need the module import for that line, won't work in this scenario.

[Ebag333]

One thing that I'd like for you to look at @blitzmann.

service.market.py references stuff from conversions, but the way it does it is weird. Can you take a look at this? (Line 265)

I totally forgot about this until I started going back through the code looking at TODOs.

[blitzmann]

Gonna make a list of things that need to be fixed, just as a reminder to do them. I have the merged version on test_import branch, which is based on dev branch (these issues could totally be bad merges on my part v0v)

• FIXED CREST Fit import causes traceback:

Traceback (most recent call last):
  File "C:\Users\Ryan\Sync\Git\blitzmann\Pyfa\gui\crestFittings.py", line 156, in importFitting
    fits = sFit.importFitFromBuffer(data)
AttributeError: 'Fit' object has no attribute 'importFitFromBuffer'
Traceback (most recent call last):
  File "C:\Users\Ryan\Sync\Git\blitzmann\Pyfa\gui\crestFittings.py", line 156, in importFitting
    fits = sFit.importFitFromBuffer(data)
AttributeError: 'Fit' object has no attribute 'importFitFromBuffer'

• FIXED CREST export broken as well:

Traceback (most recent call last):
  File "C:\Users\Ryan\Sync\Git\blitzmann\Pyfa\gui\crestFittings.py", line 273, in exportFitting
    data = sFit.exportCrest(fitID)
AttributeError: 'Fit' object has no attribute 'exportCrest'

• FIXED Graphing fails with the following message: "Problems importing matplotlib; continuing without graphs". And then trying a second time gives

Traceback (most recent call last):
 File "C:\Users\Ryan\Sync\Git\blitzmann\Pyfa\gui\mainFrame.py", line 922, in openGraphFrame
   self.graphFrame.SetFocus()
 File "C:\Users\Ryan\Sync\WinPython-32bit-2.7.10.1\python-2.7.10\lib\site-packages\wx-3.0-msw\wx\_core.py", line 10129, in SetFocus
   return _core_.Window_SetFocus(*args, **kwargs)
TypeError: in method 'Window_SetFocus', expected argument 1 of type 'wxWindow *'

• FIXED Ctrl+C > XML export is broken

Traceback (most recent call last):
  File "C:\Users\Ryan\Sync\Git\blitzmann\Pyfa\gui\mainFrame.py", line 722, in exportToClipboard
    CopySelectDict[selected]()
  File "C:\Users\Ryan\Sync\Git\blitzmann\Pyfa\gui\mainFrame.py", line 695, in clipboardXml
    fits = map(lambda fitID: db.getFit(fitID), fitIDs)
TypeError: argument 2 to map() must support iteration

[Ebag333]

I have the merged version on test_import branch, which is based on dev branch.

Do you want me to push commits against the new branch or against this PR?

[blitzmann]

Do you want me to push commits against the new branch or against this PR?

I actually just pushed it to your repo so you can make any changes necessary. :) I'll continue to test

[Ebag333]

Help help I've been hacked!

[Ebag333]

CREST Fit import causes traceback:

Fixed. Was caused by moving the function to Port and not catching that the CREST import/export used it. Not sure why pyCharm missed that it's a broken reference.

CREST export broken as well:

Same as above

Graphing fails with the following message: "Problems importing matplotlib; continuing without graphs". And then trying a second time gives

Likely been broken this way a long time. This is an issue with the way we handle the graph window, see #952

I did clean up the super fugly imports in the middle of the file though, so we have better logging if it's not imported.

Ctrl+C > XML export is broken

Not sure how what I did with this would have caused this. Literally just ported it over to Port from Fit, structurally and code wise it didn't change.

Anyway, this is fixed, for some reason we were using a lamba to create an iterable instead of simply stuffing it into a set. So the stack trace has been resolved, and as an added bonus it's a bit simpler. :)

[blitzmann]

Another one when trying to backup all fittings:

FIXED

Traceback (most recent call last):
  File "C:\Users\Ryan\Sync\WinPython-32bit-2.7.10.1\python-2.7.10\lib\threading.py", line 810, in __bootstrap_inner
    self.run()
  File "C:\Users\Ryan\Sync\Git\blitzmann\Pyfa\service\port.py", line 1137, in run
    backedUpFits = sFit.exportXml(self.callback, *allFits)
AttributeError: 'Fit' object has no attribute 'exportXml'

FIXED
And when trying to file > export fit

Traceback (most recent call last):
  File "C:\Users\Ryan\Sync\Git\blitzmann\Pyfa\gui\mainFrame.py", line 400, in showExportDialog
    output = sFit.exportXml(None, self.getActiveFit())
AttributeError: 'Fit' object has no attribute 'exportXml'

[blitzmann]

FIXED Import fitting is broken:

Traceback (most recent call last):
  File "C:\Users\Ryan\Sync\Git\blitzmann\Pyfa\gui\mainFrame.py", line 776, in fileImportDialog
    sFit.importFitsThreaded(dlg.GetPaths(), self.fileImportCallback)
AttributeError: 'Fit' object has no attribute 'importFitsThreaded'

[Ebag333]

Import fitting is broken:

Fixed.

pyCharm doesn't seem to catch these broken references when they're threaded. One more reason to not use threading....

[blitzmann]

merged into dev branch, tested on all platforms