Skip to content

Commit

Permalink
feat: multiple improvements from kfsone
Browse files Browse the repository at this point in the history
* chore: increase the line limit in the .editorconfig

* chore: relax tox warnings by disabling spaces-after-:

* refactor: Cleanup pass on cache and spansh_plug

- introduce type annotations,
- Small performance considerations,

* refactor: flake and pylint rules

- ignore warnings that are overly opinionated for our needs,
- tell pylint to quit complaining about lines > 100

* fix: ensure db gets closed before trying to rename it.

* perf: modernize small performance factors in spansh

- lookup systems and stations by id rather than name,
- move several 'is it registered' tests out of ensure_ methods
  so we can avoid calling the method if there is no work to do
  (calling a function in python is expensive)
- fix ingest_stream looking up body ids rather than system ids

* chore: remove vestigial numpy stuff that annoyed linters

I couldn't see any evidence of this code still being useful.

* chore: move uprint into a mixin

tradeenv was already hard enough to read, but the utf8 wrapping of uprint made it worse, and less pythonic.

this change moves the io behavior into a base Mixin class, then provides the non-utf8 variant as an optional override mixin. finally, we do a simple if on which one to use as the favored implementation to mixin to TradeEnv.

This should be far more readable to both humans and linters etc. At least, on my machine, PyCharm no-longer catches fire.
(* it's possible that it never caught fire and that I just ate an overly hot chilli last night, but who are we to question the universe? not deflecting enough, well LOOK A SQUIRREL)

* chore: appease the gods of pylint

pylint gives deeper code warnings than flake8 can, but is slower and more spammy.

* chore: change terribly misnamed 'str' to 'text'

* chore: tox/pylint: now we've appeased, lets ask for more

this will enable some more useful pylint warnings/messages, and also making it think a little harder about others so they're more useful.

* feat: introduce theming in tradeenv

two fold reasoning:
1- so people can opt out of any kind of decoration if it becomes inconvenient/problematic,
2- localization and style

In particular: the colors people want for light-vs-dark background terminals might vary, and this would make it easy to switch; second, this makes it possible to configure various strings to be localizable through the theme.

The demo-case included here is that when you use --color the word "NOTE" will be replaced with the "information-source" emoji and the word "WARNING" will be replaced with a warning-symbol emoji, the '#' for bugs will be replaced with a bug emoji.

* chore: cleanup/performance tradedb.py

this eliminates some dead methods, and switches some of the math operations to their favored modern-python/performant/64-bit versions

specifically:

%timeit 2 ** 2
94ns
%timeit 2 * 2
24ns

Also, when I originally feared math_sqrt vs ** it was probably because I didn't realize that "math.sqrt" was having to lookup math and sqrt each time it was called, duh.

* feat: spansh import presentation

- Adds Progresser to spansh_plugin as an experimental ui presentation layer using rich,
-- active progress bars with timers that run asynchronously (so they shouldn't have a significant performance overhead),
-- opt-out to plain-text mode incase it needs turning off quickly,
- present statistics with the view to give you a sense of progress,
- small perf tweaks

* style: blank lines have same indent as following non-blank line

* fix: Use same supply/demand levels as before

* fix: having the fdev_id listed as a unique column causes dump prices to break.

* refactor: tox was configured to give flake8 its own environment

The idea is that flake8 is really fast, so you give it it's own environment and it comes back and says "YA MADE A TYPO OLIVER YA DID IT AGAIN WHY YOU ALWAYS..." *cough*, sorry, anyway.

I usually run tox one of two ways:

```
tox -e flake8   # for a quick what-did-I-type-wrong check
tox --parallel  # run em all, but, like, in parallel so I don't retire before you finish
```

* test: flake8 appeasement

turning on flake8 made linters very shouty

* refactor: tox.ini

flake8 now runs its own environment that is JUST flake8 so it's fast, it doesn't install the package for itself;
added lots of flake8 ignores for formatting issues I'm unclear on

* chore: flake8 warnings

* refactor: tox.ini

added a lot more ignores and some file-specific ignores, but generally got it to a point where flake8 becomes proper useful.

* chore: fix all the tox warnings

This fixes all of the tox warnings that I haven't disabled, and several that I did.

* chore: bump pylint score up to 9.63/10

fixed a few actual problems while I was at it.

* fix: formatting needs __str__ methods

When I first created the 'str' methods, it was because I wanted a display name but repr and str both confused me at the time; I've now renamed it text but apparently I forgot that in Py2.x def str() worked like __str__...

* refactor: use ijson instead of simdjson to reduce memory use in big imports

* refactor: logic reversal for plugin option handling

* fix: presentation was a bit wonky with doubled progress bars

---------

Co-authored-by: eyeonus <eyeonus@panther>
Co-authored-by: Jonathan Jones <[email protected]>
  • Loading branch information
3 people authored Apr 24, 2024
1 parent 1801216 commit 7c60def
Show file tree
Hide file tree
Showing 82 changed files with 1,796 additions and 1,528 deletions.
1 change: 1 addition & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ charset = utf-8
indent_style = space
indent_size = 4
trim_trailing_whitespace = false
max_line_length = 160

# Tab indentation (no size specified)
[Makefile]
Expand Down
2 changes: 1 addition & 1 deletion requirements/dev.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@ flake8
importlib-metadata>=1
wheel
coverage
pysimdjson~=6.0.2
ijson
rich==13.7.1
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

setup(name = package,
version = __version__, # pylint: disable=E0602
install_requires = ["requests", "appJar", "pysimdjson", "rich"],
install_requires = ["requests", "appJar", "ijson", "rich"],
setup_requires = ["pytest-runner"],
tests_require = ["pytest"],
packages = ['tradedangerous', 'tradedangerous.commands', 'tradedangerous.mfd', 'tradedangerous.mfd.saitek', 'tradedangerous.misc', 'tradedangerous.plugins'],
Expand Down
2 changes: 1 addition & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def pytest_addoption(parser):
def pytest_collection_modifyitems(config, items):
has_runslow = config.getoption("--runslow")
has_runsuperslow = config.getoption("--runsuperslow")

skip_slow = pytest.mark.skip(reason="need --runslow option to run")
skip_superslow = pytest.mark.skip(reason="need --runsuperslow option to run")
for item in items:
Expand Down
3 changes: 1 addition & 2 deletions tests/helpers.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import sys
import os
import re
from time import sleep
from io import StringIO
from pathlib import Path
from contextlib import contextmanager
Expand All @@ -12,7 +11,7 @@
__all__ = ['tdenv', 'captured_output', 'is_initialized']

_ROOT = os.path.abspath(os.path.dirname(__file__))
_DEBUG=5
_DEBUG = 5
tdenv = TradeEnv(debug=_DEBUG)

@contextmanager
Expand Down
4 changes: 1 addition & 3 deletions tests/test_bootstrap_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ def test_import_commands(self):
self.assertIn('buildcache', commands.commandIndex)
self.assertIn('buy', commands.commandIndex)


def test_import_buildcache_cmd(self):
from tradedangerous.commands import buildcache_cmd

Expand Down Expand Up @@ -47,7 +46,6 @@ def test_import_station_cmd(self):

def test_import_update_cmd(self):
from tradedangerous.commands import update_cmd

def test_import_update_gui(self):
from tradedangerous.commands import update_gui

2 changes: 1 addition & 1 deletion tests/test_bootstrap_plugins.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import pytest

class TestBootstrapPlugins(object):
class TestBootstrapPlugins:
def test_import_traded(self):
import tradedangerous as td

Expand Down
2 changes: 1 addition & 1 deletion tests/test_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

FakeFile = namedtuple('FakeFile', ['name'])

class TestCache(object):
class TestCache:
def test_parseSupply(self):
fil = FakeFile('faked-file.prices')
reading = '897H'
Expand Down
8 changes: 4 additions & 4 deletions tests/test_commands.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
#! /usr/bin/env python
# pytest

from __future__ import absolute_import, with_statement, print_function, division, unicode_literals
import sys
import pytest
from tradedangerous import commands
from tradedangerous.commands.exceptions import UsageError, CommandLineError
Expand All @@ -11,9 +9,11 @@
def cmd():
return commands.CommandIndex()


prog = "trade.py"

class TestCommands(object):

class TestCommands:

def test_dashh(self, cmd):
with pytest.raises(UsageError):
Expand All @@ -39,4 +39,4 @@ def test_local_validsys(self, cmd):
cmd.parse([prog, 'local', 'ibootis'])

def test_local_validsys_dashv(self, cmd):
cmd.parse([prog, 'local', 'ibootis', '-v'])
cmd.parse([prog, 'local', 'ibootis', '-v'])
5 changes: 3 additions & 2 deletions tests/test_fs.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def tdenv():
return TradeEnv()


class TestFS(object):
class TestFS:

def test_copy(self, tdenv):
src = fs.pathify(tdenv.templateDir, 'TradeDangerous.sql')
Expand All @@ -35,7 +35,8 @@ def action():
self.result = True

flag = fs.ensureflag(flagfile, action)
assert self.result == True
assert self.result is True
assert flag is not None

def test_copyallfiles(self, tdenv):
setup_module()
Expand Down
25 changes: 12 additions & 13 deletions tests/test_peek.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
import os
import random

import pytest

from tradedangerous.tradedb import Trade, TradeDB, Station, System
from tradedangerous.tradedb import TradeDB, Station, System
from .helpers import copy_fixtures

ORIGIN='Shinrarta Dezhra'
#tdb = None
ORIGIN = 'Shinrarta Dezhra'
# tdb = None

def setup_module():
copy_fixtures()

def route_to_closest(tdb:TradeDB, origin, destinations, maxLy=15):
def route_to_closest(tdb: TradeDB, origin, destinations, maxLy=15):
closest = min(destinations, key=lambda candidate: candidate.distanceTo(origin))
print("Closest:", closest.name(), closest.distanceTo(origin))
route = tdb.getRoute(origin, closest, maxLy)
Expand All @@ -23,16 +22,16 @@ def route_to_closest(tdb:TradeDB, origin, destinations, maxLy=15):
return route

def should_skip() -> bool:
return False # os.getenv("CI") != None
return False # os.getenv("CI") != None


class TestPeek(object):
class TestPeek:
"""
Tests based on https://github.com/eyeonus/Trade-Dangerous/wiki/Python-Quick-Peek
"""

@pytest.mark.skipif(should_skip(), reason="does not work with CI")
def test_quick_origin(self, tdb:TradeDB):
def test_quick_origin(self, tdb: TradeDB):
# Look up a particular system
origin = tdb.lookupSystem(ORIGIN)

Expand Down Expand Up @@ -70,7 +69,7 @@ def test_quick_lookupPlace(self, tdb):

abe = tdb.lookupPlace("sol/hamlinc")
assert isinstance(abe, Station)

@pytest.mark.skipif(should_skip(), reason="does not work with CI")
def test_quick_five(self, tdb):
systemTable = tdb.systemByID.values()
Expand All @@ -89,7 +88,7 @@ def test_quick_five(self, tdb):
else:
# Route is a list of Systems. Turn it into a list of
# System names...
routeNames = [ system.name() for system, distance in route ]
routeNames = [system.name() for system, distance in route]
print("Route:", routeNames)

route_to_closest(tdb, origin, visitMe)
Expand All @@ -106,5 +105,5 @@ def test_three_different(self, tdb):
lhs = tdb.lookupSystem("lhs 380")
bhr = tdb.lookupSystem("bhritzameno")

result = route_to_closest(tdb, sol, [ lhs, bhr ])

route_to_closest(tdb, sol, [lhs, bhr])

4 changes: 2 additions & 2 deletions tests/test_tools.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import pytest
import pytest # noqa: F401

from tradedangerous import tools
from .helpers import copy_fixtures
Expand All @@ -8,6 +8,6 @@ def setup_module():
copy_fixtures()


class TestTools(object):
class TestTools:
def test_derp(self, capsys):
tools.test_derp()
20 changes: 10 additions & 10 deletions tests/test_trade.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,39 +15,39 @@ def teardown_module():
remove_fixtures()


class TestTrade(object):
class TestTrade:
def test_local_help(self):
with pytest.raises(UsageError):
trade([PROG, "local", "-h"])

def test_local_sol(self, capsys):
trade([PROG, "local", "--ly=10", "--detail", "sol"])
captured = capsys.readouterr()
assert "Sol 0" in captured.out
assert "Ehrlich City" in captured.out

def test_sell(self, capsys):
trade([PROG, "sell", "--near=sol", "hydrogen fuel"])
captured = capsys.readouterr()
assert "Sol/Mars High" in captured.out

def test_buy(self, capsys):
trade([PROG, "buy", "--near=sol", "hydrogen fuel"])
captured = capsys.readouterr()
assert "Cost Units DistLy Age/days" in captured.out

def test_export_station(self, capsys):
trade([PROG, "export", "-T", "System"])
captured = capsys.readouterr()
assert "NOTE: Export Table 'System'" in captured.out
# TODO: check that System.csv has a fresh date

def test_station_remove(self, capsys):
# "Dekker's Yard"
trade([PROG, "station", "-rm", "sol/dekkers"])
captured = capsys.readouterr()
assert regex_findin(r"NOTE: Sol/Dekker's Yard \(#\d+\) removed", captured.out)

def test_station_add(self, capsys):
# "Dekker's Yard"
trade([
Expand All @@ -63,20 +63,20 @@ def test_station_add(self, capsys):
"sol/Dangerous Delight"])
captured = capsys.readouterr()
assert regex_findin(r"NOTE: Sol/Dangerous Delight \(#\d+\) added", captured.out)

def test_nav(self, capsys):
trade([PROG, "nav", "--ly-per=50", "sol", "Shinrarta Dezhra"])
captured = capsys.readouterr()
assert "System JumpLy" in captured.out
assert "Shinrarta Dezhra 47" in captured.out

def test_market(self, capsys):
trade([PROG, "market", "sol/abr"])
captured = capsys.readouterr()
assert regex_findin("Item[ ]{3,}Buying Selling[ ]{2,}Supply", captured.out)
assert "Hydrogen Fuel" in captured.out
assert regex_findin("Water[ ]{3,}323", captured.out)

@pytest.mark.slow
def test_import_edcd(self, capsys):
trade([PROG, "import", "-P=edcd", "--opt=commodity"])
Expand Down
2 changes: 1 addition & 1 deletion tests/test_trade_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def setup_module():
copy_fixtures()


class TestTradeRun(object):
class TestTradeRun:
def test_run1(self, capsys):
trade([PROG, "run", "--capacity=10", "--credits=10000", "--from=sol/abr", "--jumps-per=3", "--ly-per=10.5", "--no-planet"])
captured = capsys.readouterr()
Expand Down
4 changes: 2 additions & 2 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import pytest
import pytest # noqa: F401

from tradedangerous import utils
from tradedangerous import TradeEnv


class TestUtils(object):
class TestUtils: # should inherit from TestCase
# TODO: Test 'von' etc.

def test_titleFixup_s(self):
Expand Down
Loading

0 comments on commit 7c60def

Please sign in to comment.