mikemalinowski / qute Goto Github PK

os.path.isdir() will check if the path exists AND if its a directory.
Depending on usage, it might be "safer" to use than os.path.exists().

Personally, I let the layout constraints and affecting size policies do there thing, only setting the size if I need to.
Forcing a size argument feels a little unnecessary outside of the button's existing usage?

I've hit a few issues before when tokenizing a string when I didnt run str.strip() on each token.
Unsure if os.path.exists will care, but if so, then you may need to do it twice.

Unsure why the package imports using a different namesapce?
If related to backwards compatibility, it might be clearer/cleaner to use aliases later on?
I can imagine a wide usage of both names depending on where its being imported from, which might cause unnecessary confusion.

It might be worthwhile ensuring you know exactly what is being imported from _core by providing an __all__ in the module.
This will avoid you accidentally bringing over anything unintentionally.

Depending on usage, builtin set might be a viable type to also accept?
That being said, the method name certainly might argue otherwise...

If desired, it might be useful to support iterables in value.
Paths could be joined with os.pathsep
https://docs.python.org/3/library/os.html#os.pathsep

Text could be joined with newlines.

dicts could be encoded with json or cast as str() (its a stretch but sometimes useful).

I've found this pattern useful in such a generic function, although it does make some assumptions.

Currently qApp() will swallow any *args or **kwargs given - I'm not sure why?
QApplication can be given arguments for initialisation, if its meant for them should they be passed?

types imported twice.

Not sure of the benefit of catching all exceptions, but except Exception is usually intended instead.
I would be interesting in understanding the benefit in this case if this is desirable.

Currently implicitly decides that its using python 2 or 3 based on available imports.
As unlikely as it is, you may have matching available module names or identically named backports that could cause issues with the assumption that you're running on Python 2/3.

It might be "safer" to explicitly check the version of python and import from that?

It might be safer to ensure there is a root logger available. If there isnt byt this point , I dont think the log object will output.

Might be worth double checking if you need to call logging.basicConfig(level=logging.INFO) beforehand, perhaps ideally with your preferred log level. Its not used if a root logger already exists, so there shouldnt be a downside.

https://docs.python.org/3/library/os.html#os.pathsep

It may improve os compatibility.

The six module can be useful for instances like this.
For example, in this method, you're always unnecessarily paying the cost of testing against basestring if you're in Python 3.

Other option is to define the missing type once;

try:
    basestring
except NameError:
    basestring = str

and then using basestring.
This being said, it may also cause confusion :/

AFAIK, setting a global (right terminology?) stylesheet on a QObject may unintentially reduce flexibility from inherited styles.
par = QWidget() par.setStyleSheet('QFrame:hover{border: 2px solid green;}') div = HorizontalDivider(parent=par)

That being siad, its quite possible this is the intended behaviour (if so, feel free to close).