# Tag Info

4

get_all_ingredients List comprehensions are very pythonic and often times faster (especially for operations, that aren't computationally expensive), so instead of answer = "" for key, value in self.ingredients.items(): answer += f"{key} ({value.get('abv')}%)\n" you should use answer = "".join(f"{key} ({value.get('abv')}...

3

Instance-ness Your file reading is done in class static scope. It should be moved to class instance scope. Type hints Add some PEP484 type hints to your function signatures. Type soundness and dictionary soup Once you've read in your data from JSON, you should not leave it as dictionaries - make actual classes so that your data have stronger correctness ...

3

General review points Use semicolons Try to avoid Array iterator functions in favor of for of. Generally use array iterators when you want the iteration to be an expression. Apart from that the code looks good and is an elegant solution to parsing simple markup. It can be used as a base to provide the additional functionality you mention. Questions "...

2

A couple comments: _is_convertible_to_str() As you discovered, the second argument to isinstance() can be a tuple of classes: def _is_convertible_to_str(value: Any) -> bool: return isinstance(value, (str, int, float)) You can call str() on just about anything, so _is_convertable_to_str() isn't very descriptive. I'd drop the _is_convertible_to_str() ...

2

You need to get rid of your "payload" antipattern. The dictionary you're constructing isn't particularly a payload; it's just a grab bag of untyped, unorganized data. If not OOP, then at least put this in normal variables; but avoid a dictionary unless you're actually doing something that requires a dictionary - e.g. saving to a JSON file, or ...

2

You need functions. Functions help to split up your code and make understanding how everything fits together easier. You should have a function for: Getting the page. Parsing the JSON with your helpful error message. Getting the payload. A main function for calling all the other functions. We should only call the main function if the script is the entry ...

1

Changes: Quote variables unless you have a good reason not to quote them. How about a password * $(touch /tmp/sorry) * ? Use${parameter:-word} for turning an empty vaultenv into prod. If parameter is unset or null, the expansion of word is substituted. Otherwise, the value of parameter is substituted. Translate uppercase to lowercase only once. Use \${...

1

I agree with your assessment here; isinstance(value, _CONVERTIBLE_TO_STR) (where _CONVERTIBLE_TO_STR = (str, int, float)) is simpler and better than _is_convertible_to_str(value). It is also a little bit faster. I determined this by measuring execution times with timeit. _contains_any looks like it was written in a way so it can be useful in a general sense, ...

1

If you need to account for mixed response types (in your case "unknown errors" or JSON-formatted errors), then you could consider checking the Content-Type of the response. See this StackOverflow answer for reference. This, of course, trusts in the server echoing the correct content type header for a response. As you mentioned, fetch does not ...

1

Keys names, and start and end indices do not ever change. It makes no sense to test them at each iteration. Move all the error checking fragments outside of the loop. Same applies to content slicing. Compute the boundaries beforehand. The first line of your data file is aligned differently. I bet its json is incorrect. Instead of slicing content at columns, ...

Only top voted, non community-wiki answers of a minimum length are eligible