Added documentation for app.paths #1849
Conversation
There was a problem hiding this comment.
A good start; some suggestions inline. I'd also suggest running through the development environment setup in the docs; that will catch some of these markup errors before you submit a PR. Those docs also describe why the towncrier CI failure is occurring.
core/src/toga/app.py
Outdated
| resizeable=True, | ||
| minimizable=True, | ||
| factory=None, # DEPRECATED ! | ||
| on_close=None, |
There was a problem hiding this comment.
I'm betting you use PyCharm. I can tell, because PyCharm's default indentation rules are Just Wrong. This has been picked up by the pre-commit check run in CI; if you install pre-commit in your local development environment, you'll get the same warning.
core/src/toga/app.py
Outdated
| locations. | ||
|
|
||
| PROPERTIES: | ||
| **App**: Location of where the apps code is run. |
There was a problem hiding this comment.
The capitalisation is wrong here - the property is app, not App.
core/src/toga/app.py
Outdated
|
|
||
| **Data**: System appropriate location to store user data. | ||
|
|
||
| **Cache**: System cache location. Files stored in this location may be able to be accessed quicker than normal locations. |
There was a problem hiding this comment.
This is a bit misleading. Cached files are used to optimise runtime performance, but that's not because the files load faster - cache file locations are just file locations. They're no faster to access.
The distinction is more about whether the files should be considered a permanent store of data, or ephemeral. You should assume that any cached file can and will be purged by the operating system at any time. Your browser uses cached files so that it doesn't need to repeatedly download an asset file; but if the cached file is deleted, it can be restored by downloading the file again.
core/src/toga/app.py
Outdated
|
|
||
| **Logs**: Location of toga log files. | ||
|
|
||
| **Toga**: Returns a path to a Toga system resources. |
There was a problem hiding this comment.
It would be more accurate to say the location of the core Toga code module.
core/src/toga/app.py
Outdated
| Paths for known safe system locations. Using paths makes sure you have a consistent and known location to store | ||
| files for your app. Also, some platforms do not allow arbitrary file access to any location on disk. On Android, | ||
| there are many limitations when it comes to accessing file locations and the apis you can use to read from those | ||
| locations. |
There was a problem hiding this comment.
Line length should be wrapped at ~80 chars. This should also mention that the locations returned are Pathlib objects.
core/src/toga/app.py
Outdated
| there are many limitations when it comes to accessing file locations and the apis you can use to read from those | ||
| locations. | ||
|
|
||
| PROPERTIES: |
There was a problem hiding this comment.
It would be better to use native ReST markup, rather than capitalisation and indentation. This is effectively a subheading with bullet point descriptions.
It probably also warrants a description of why a property (paths) has properties.
Changed definition listed for cache and toga properties Updated formating to use ReST styles
|
Realized I included the wrong information in the changes file. Will update that to the proper information if anything else needs changed then or if it's all good I will submit the updated file. |
core/src/toga/app.py
Outdated
| @property | ||
| def paths(self): | ||
| """ | ||
| Paths for known safe system locations. Using paths makes sure you have a consistent and known location to store |
There was a problem hiding this comment.
Line length is still an issue - it should be wrapped at ~80 chars.
There's also no mention of Pathlib as a return type, as previously requested.
There was a problem hiding this comment.
Can I instead suggest that we wrap at 88 columns, the same as Black? That makes it much easier to set one line length in your editor and use it everywhere.
core/src/toga/app.py
Outdated
| """ | ||
| Paths for known safe system locations. Using paths makes sure you have a consistent and known location to store | ||
| files for your app. Also, some platforms do not allow arbitrary file access to any location on disk. | ||
| On Android, there are many limitations when it comes to accessing file locations and the apis |
There was a problem hiding this comment.
This isn't an Android-specific thing. iOS has the same set of constraints - and technically, so do macOS and Windows when you use packaged/sandboxed apps.
Also - it's not just that they're known locations - it's that they're platform appropriate locations.
Co-authored-by: Russell Keith-Magee <russell@keith-magee.com>
… set to 100 and not 80 like I thought. Added description for why paths has properties Added return type Removed note about android specific limitations.
Created a paths property so that it would be added to the app API Reference.
Added documentation for app.paths.
Adds documentation for app.paths so that users know how to use app.paths
Fixes #1569
PR Checklist: