Hot answers tagged

14

I would suggest to separate the concerns of coloring and animating. Now, in particular theater_chase_rainbow duplicates code from other methods. Instead, have theater_chase take a color array as parameter. If you then add some animation styles and color schemes you can have \$O(n^2)\$ amount of fun trying all combinations. It seems to me that in this code i +...


12

Style Python has a style guide. It's called PEP8, and normally if you follow it you get easier to read code. It's a fairly small read, and you can learn some new things from it. It's recommended in PEP8 to limit the character width of scripts to 79 characters. It has a few benefits: People on StackExchange don't need to use the horizontal scroll. It's ...


11

Models - General The comments on the model definitions don't add much value. I could figure out the relationships from the db schema; tell me instead something about what the classes are meant to represent, any invariants they have, anything not made explicit in the code. Models - User When defining a backreference, as you do in User, I like to go to the ...


8

Your imports and module structure seems like an incredible mess. Yet your application only contains 3 files. Let's try and improve that. Value your namespaces import this Preferably in an interactive session, will give you an insight of how you should approach problems in Python. In your case, it is the last line Namespaces are one honking great idea -- let'...


8

Structure Nothing! The code seems fairly straightforward and clear, and I was able to understand what you’re doing even if I didn't study flask too much. So I don’t think there’s anything wrong with the structure. Code style One small nitpick: some of your lines are too long (>79 characters). From PEP8: Limit all lines to a maximum of 79 characters. ...


7

Disclaimer: I haven't done authentication in Flask before. Tl;dr: Don't do authentication yourself, it's VERY hard to do right (and using a global ain't right); also, separate the user's authentication from their key store: the former you should ONLY be able to verify, not decode, and the latter should be inaccessible to your server without some secret ...


6

the idea is nice, but I have few concerns. Firs of all, trying to call importlib every time you are accessing the API endpoint is a bit expensive. It will be a way better if you will introduce some cache first. You can use 2 approaches to build that cache: Lazy, where you lookup cache first and if you cannot find the proper handler, you are trying to load ...


6

Exception itself actually inherits from a class called BaseException so: class BaseException(Exception): Seems backwards.


6

As Reinderien mentions, you can decrease some of the complexity of your code by inverting your if-statements. While I cannot test this because I do not have your whole code, here is how it could be done: def get_image(width, height, collection_sku, order_id, name, handle): """image processing pipeline""" # Check-1 image exception should be checked ...


5

You don't need dynamic type creation I don't quite understand why you're creating new types dynamically. I think you just need to define a constructor for your FindExact or DictionaryResource classes, and then pass the keyword argument resource_class_args to add_resource: resource_class_args (tuple) – args to be forwarded to the constructor of the ...


5

You could break your main function into multiple functions. Doing that will not even make main() much more clearer but also you will get some re-useable functions. Having these tiny functions allow you to mock them easily in tests and also allows them to be tested them separately. I have also added response.raise_for_status() calls to raise an HTTPError if ...


5

Document with docstrings. Your functions and ORM models are currently not providing any useful documentation regarding their respective function within your program. Don't initialize the database on module level: You should put that into a function: Base.metadata.create_all(engine) Base.metadata.bind = engine Session = (sessionmaker(bind=engine)) #...


5

At the least, this can be done more efficiently: if k == 'update_name_first': user.name_first = v.rstrip() if k == 'update_name_last': user.name_last = v.rstrip() if k == 'update_name_company': user.name_company = v.rstrip() if k == 'update_email': ...


4

Building on @Drewverlee's answer, I would consider renaming BaseException to APIException, or something of the like. Also, I don't think passing app as an argument is best practice, I would recommend using the function as a decorator on your endpoints instead: # Your imports from application import app # Or whatever your non-relative reference is def ...


4

It's a bit difficult to read this code, because it violates many Pythonic coding practices. The base url http://api.steampowered.com/ISteamUser is duplicated in many places. It would be better to extract to a global variable. You might want to do likewise to all the API urls, for example: API_BASEURL = 'http://api.steampowered.com/ISteamUser' ...


4

Just because you don't have many people using this API now, this does not mean that this will always be the case. In addition, you might want to discriminate between the users of the API for various reasons: To know how many there actually are To know which user/application is generating all those requests suddenly To rate-throttle some of them if needed ...


3

Move the os.environ and sys.path calls into a new setUpClass method instead of have them at the global level? pass at the end of setUp should be removed, similarly the commented out print. I'd put the constants for the URL and the headers into fields or methods (self.url() / self.url) to reduce duplication. The docstrings are a bit much. Ideally the code ...


3

Everything is good by this point but I'd change the relation between posts and tags from one-to-many to many-to-many. You can do this with this code: tags_to_posts = db.Table( "tags_and_posts", db.Column( "post_id", db.Integer, db.ForeignKey( "posts.id" ) ), db.Column( "tag_id", db.Integer, db.ForeignKey( "tags.id" ) ) ) class Post(db.Model): ...


3

How about this? def log_exception(self, exc_info): msg = '' msg += 'Time: %s\n' % datetime.now() msg += 'Path: %s\n' % request.path msg += 'HTTP Method: %s\n' % request.method msg += 'POST_ARGS: %s\n' % request.form msg += 'GET_ARGS: %s\n' % request.args msg += '...


3

You may want to use a different template for error messages. You can still reuse the overall design by inheriting from a base template. You replace variable values a lot, making it hard to follow what the meaning of variables like num, number and strnumber are throughout the code. Instead write functions which create the various transformations of the input ...


3

In this code, at first I didn't see the "danger" until I scrolled to the right by accident: def menu(parent_id=0, menutree=None): menutree = menutree or [] cur = g.db.execute('select id, parent, alias, title, ord from static where parent="'+ str(parent_id) +'" and ord>0 order by ord') fetch = cur.fetchall() if not fetch: return ...


3

Opening files using with It's not a great idea to open a file just using something like this: dtd = open(Config.DTDPATH, 'r') If your program unexpectedly exits, or crashes, resources that are taken up by the opened file aren't freed. This can be bad in some situations. The proper way to open a file would to be using a context manager, or with, like this: ...


3

The MongoClient was designed to be instantiated once per app life cycle, so this is redundant. You should try to reuse MongoClient instances as much as possible. Basically the only reason to create a new instance is if you want to configure it in a different way. Also you've mentioned your API is supposed to be RESTful, so a few notes here: 1) To deal with ...


3

You probably want to move the session to a global variable(or simply use requests.get) else at function level it is not helping in re-using any connections right now and is adding an extra level of indentation to your code. This is mostly opinionated but grouping from x import y based imports below import z makes the imports bit easier to read. Plus keep the ...


3

Well, I shall start from the top. According to the style guide, PEP 8 imports should be in the following order: standard library imports related third party imports local application/library specific imports Plus, add a single blank line to separate each group. So, something like the following: import subprocess import threading import logging ...


3

There are a few things bothering me in one version or an other: the error=error or None is unnecessary as error is either assigned 'Invalid Email or Password' or None, stick to the simpler error=error; the ): on its own line in the third version is more than disturbing, it feels like a syntax error and force the reader to go through the code again to ...


3

Date formatting self.date_created = f"{now.month}/{now.day}/{now.year} {now.hour}:{now.minute}:{now.second} {end}" should be from datetime import datetime ... datetime.now().strftime('%m/%d/%Y %I:%M:%S %p') Generators for logic for word in self.content.split(): if word.lower() in ["phooey", "shucks", "rascal"]: return True ...


3

Few optimization tips verify_password function avoid bare except: clause - at least use except Exception: (though it's also broad, check which exception class is most appropriate in that context) a set of statements: try: if check_password_hash(user["password"], password): return True except: return False return False is an overloaded and ...


3

URL URL components should generally appear from "broadest to narrowest" left-to-right. I'm not totally convinced that this is currently the case - the weather site is probably "broader" than the latitude and longitude. Is the city within a climbing area, or vice versa? If a climbing area is within (or smaller than) a city, their positions ...


3

Relays Your current design: fetches data from alphavantage once, on startup offers it to anyone who hits any of your interfaces on port 5000 (which, by the way, is already used by a large number of services other than the default Flask development webserver) This seems unusual. Perhaps it's just to try out a random use of Flask for learning purposes, which ...


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