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

Cleanup of EDMarketConnector.py #754

Merged
merged 17 commits into from
Oct 12, 2020
Merged

Conversation

Athanasius
Copy link
Contributor

The main aim here is to get it passing my pre-commit hooks, so flake8 and mypy primarily.

I'm leaving TODO's behind on some things for later cleanup.

* Removed a couple of E501 ignores tagged as now un-necessary.
* docstring indent on Class A.
* Minor white space adjustments.
This smells of something added for debugging that can be better done
under a proper debugging setup.
This allows the type to be known, but leaves the proper setting of the
value until later (after UI creation).
* Ignore Tkinter 'name' complaints.  I've opened
  <python/typeshed#4658> to get this
  fixed in typeshed.
* If checking `import update` so it's available.
* Don't annotate journal_event 'event' arg, as it's passed through Tk
  events mechanism, so let mypy infer it.
Some of our code is just cognitively complex.
* retry_for_shipyard() was only called by itself (for further retry),
  and tried to call self.eddn.export_shipyard() which doesn't even
  exist.
  I suspect this has been unused code since the EDDN code was extracted
  out into an internal plugin.
* self.drag_offset given proper typing.
* All functions have at least some typing now.  Left the 'event' Tk
  arguments bare as mypy seems to infer without complaint.
@Athanasius Athanasius marked this pull request as ready for review October 12, 2020 15:52
And added `# type: ignore` comments.

This is a mess.  On the tkinter side it *is* an int, default 0.  But
enough python stuff has assumed it's a bool for typeshed to define that
tearoff should be.  It doesn't match, mypy complains.
EDMarketConnector.py Outdated Show resolved Hide resolved
EDMarketConnector.py Outdated Show resolved Hide resolved
EDMarketConnector.py Outdated Show resolved Hide resolved
EDMarketConnector.py Show resolved Hide resolved
EDMarketConnector.py Outdated Show resolved Hide resolved
EDMarketConnector.py Show resolved Hide resolved
EDMarketConnector.py Outdated Show resolved Hide resolved
@Athanasius Athanasius merged commit ab45375 into develop Oct 12, 2020
@Athanasius Athanasius deleted the cleanup/edmarketconnector.py branch October 12, 2020 16:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants