f4lco / om-parser-stw-potsdam-v2

Connect canteens of the Studentenwerk Potsdam to OpenMensa
3 stars 7 forks source link

feature/metafeeds: Support for canteen meta feeds and index feed #2

Closed kaifabian closed 6 years ago

kaifabian commented 6 years ago

This PR adds support for meta feeds (as documented in doc.openmensa.org Feed v2.1 and for a feed/canteen index (undocumented on doc.openmensa.org as of yet).

This PR creates incompatible URLs to provide a consistent naming scheme (/canteens/ for the canteen index, /canteens/<canteen>/meta for canteen metadata and /canteens/<canteen>/menu for the menu).

For both the index feed and metadata feed, a "reverse url" module (stw_potsdam/reverse.py) had to be added, that either uses a base URL derived from an environment variable (BASE_URL) or else from Flask's Request context.

kaifabian commented 6 years ago

Thanks for your comments! :) I will look into them and update my PR accordingly (probably on wednesday).

kaifabian commented 6 years ago

Better? ;)

kaifabian commented 6 years ago

Done so far. I just ran pycodestyle (former pep8) and pylint: pep8 only complained about too long lines:

stw_potsdam\canteen.py:5:80: E501 line too long (81 > 79 characters)
stw_potsdam\canteen_api.py:9:80: E501 line too long (88 > 79 characters)
stw_potsdam\feed.py:36:80: E501 line too long (84 > 79 characters)
stw_potsdam\feed.py:37:80: E501 line too long (100 > 79 characters)
stw_potsdam\feed.py:68:80: E501 line too long (147 > 79 characters)
stw_potsdam\views.py:33:80: E501 line too long (99 > 79 characters)
stw_potsdam\views.py:34:80: E501 line too long (100 > 79 characters)
stw_potsdam\views.py:65:80: E501 line too long (90 > 79 characters)
stw_potsdam\views.py:98:80: E501 line too long (107 > 79 characters)
tests\test_retrieval.py:23:80: E501 line too long (99 > 79 characters)
tests\test_retrieval.py:29:80: E501 line too long (80 > 79 characters)

pylint (with disabled warnings missing-docstring and line-too-long) gives this:

************* Module stw_potsdam.canteen_api
C: 12, 0: Argument name "it" doesn't conform to snake_case naming style (invalid-name)
C:  5, 0: standard import "from collections import namedtuple" should be placed before "import requests" (wrong-import-order)
************* Module stw_potsdam.config
W:  7, 0: Relative import 'canteen', should be 'stw_potsdam.canteen' (relative-import)
C: 13,44: Variable name "f" doesn't conform to snake_case naming style (invalid-name)
************* Module stw_potsdam.feed
R: 36,12: Consider merging these isinstance calls to isinstance(price, (str, unicode)) (consider-merging-isinstance)
************* Module stw_potsdam.views
W:  9, 0: Relative import 'feed', should be 'stw_potsdam.feed' (relative-import)
W: 10, 0: Relative import 'config', should be 'stw_potsdam.config' (relative-import)
W: 11, 0: Relative import 'canteen_api', should be 'stw_potsdam.canteen_api' (relative-import)
C: 15, 0: Constant name "app" doesn't conform to UPPER_CASE naming style (invalid-name)
C: 19, 4: Constant name "base_url" doesn't conform to UPPER_CASE naming style (invalid-name)
C: 27, 0: Constant name "cache" doesn't conform to UPPER_CASE naming style (invalid-name)
E: 31, 4: Method 'logger' has no 'warn' member (no-member)
E: 42, 8: Method 'logger' has no 'info' member (no-member)
E: 47, 4: Method 'logger' has no 'info' member (no-member)
************* Module stw_potsdam.__main__
W:  2, 0: Relative import 'views', should be 'stw_potsdam.views' (relative-import)

Since only the base_url uppercase naming is related to this PR (and this is chosen to be consistent with the surrounding code), I think these should be addressed in another PR. (The no-member errors are obviously false positives.)

If you'd like, I could also give those a try, but I would need to know which one you would want to be addressed, and probably you're quicker just fixing them directly. :wink:

f4lco commented 6 years ago

Thanks for the info, yes, I'll postpone complaints of both tools. Those will be subject to later PRs. Are you generally interested in being a reviewer? I also noticed irregularities in the canteen API which require fixing (work in progress on my side).