Better tracks documentation #275
Conversation
Codecov Report
@@ Coverage Diff @@
## master #275 +/- ##
=======================================
Coverage 84.15% 84.15%
=======================================
Files 12 12
Lines 2064 2064
=======================================
Hits 1737 1737
Misses 327 327
Continue to review full report at Codecov.
|
|
|
||
| Alternatively, a `Tracks` object can be constructed and then shown; `find_zero` modifies a `Tracks` object. | ||
| `Tracker` objects are printed in a nice, easy-to-read format. You can also | ||
| access individual fields if you need the values. A `Tracks` object has the following fields: |
There was a problem hiding this comment.
As there is a chance this could be refactored, it would be best to include the usual reminder that internal fields are subject to modification. (Maybe just say "Currently a tracks object ...")
There was a problem hiding this comment.
So "Currently a tracks object is printed in a nice, easy-to-read format..."?
There was a problem hiding this comment.
I was referring to the fieldname (tracks.xs, tracks.fs...) These may change, though unlikely, so I'm suggesting using "currently" at the point you describe these.
There was a problem hiding this comment.
I would say that the documentation should be updated if the field names change. The docs should not be uncertain about what things are called.
There was a problem hiding this comment.
Once documented, even if not exported, they become expected by users, making it harder to modify things should a refactoring be done. (This is exactly why I'm reluctant to rename Tracks, even if I agree with your take). Take for example the discussion on the need to two types of ways to record xs (one with brackets, one without). I'd like the freedom to change that without feeling responsible for mucking up someone's code.
There was a problem hiding this comment.
Fair points. The docs are allowed to become after a potential refactoring (meaning a potential 2.0, right?)
There was a problem hiding this comment.
Yes, for sure. But changing things on an old package is usually best reserved for necessary things (performance improvements...)
There was a problem hiding this comment.
I am having a hard time seing exactly what you want to change. Feel free to make the change yourself - I trust your judgment
|
Thanks, this looks pretty good and definitely necessary for anyone who wants to use this feature! I just had that one comment about reminding the reader that internal fields may be renamed, modified, ... (though in this case, it is really unlikely). |
* Add "Tracks" to reference/API * rework the Tracks object * use unicode to distinguish abs fields in Tracks * merge and edit documentation PR #275 by KronosTheLate * tidy logging methods * docfix Co-authored-by: KronosTheLate <[email protected]>
|
I had thought my merging this into PR #280 would have closed this, but it didn't. So am closing. Thanks for your help here! |
|
I thought you would show up in the contributors with how I merged this, but am wrong. I’d love it if you put in a PR to bump the version, so that you do. |
I just has a significant amount of problems navigating my way through the documentation for
Tracks. This PR significantly improves the docstring, adds a section in the documentation, and makes the reference toTracksin the docstring offind_zeroa link.Fact-checking and other improvements are most welcome.