Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addition of narrative docs instead of solely API docs #160

Merged
merged 13 commits into from
Aug 16, 2022

Conversation

ksunden
Copy link
Member

@ksunden ksunden commented Aug 1, 2022

  • Outline plus structure docs
  • Add store docs

@ksunden
Copy link
Member Author

ksunden commented Aug 2, 2022

Okay, I am rounding out the text portion of this PR...

I would like to add a short example inline for using each of the tuning workup methods (2 for intensity, one for opa tuning, one for SDC) with code and output image.

I intend to do that tonight, but will invite review on content other than example usage/request for images of those four methods.

I am particularly interested in reviews from @untzag and @ddkohler for correctness and completeness of the docs, and from @pcruzparri and @rpm4 for understandability from the perspective of people who at least mostly know what we are trying to do, but have not actually done it many times over.

@ksunden
Copy link
Member Author

ksunden commented Aug 2, 2022

I do intend on adding some of the workup scripts used in the lab as examples in their full complexity in the Gallery, but that will come as a followup PR which will likely go back and link to them from these sections.

@ksunden ksunden marked this pull request as ready for review August 3, 2022 01:20
@ksunden
Copy link
Member Author

ksunden commented Aug 3, 2022

For convenience of review I got the docs building on https://attune.wright.tools/en/docs_fill/

Which also incurred fixing the failing docs build to begin with (we use py3.7+ features and told it to build on py3.6)

@ksunden
Copy link
Member Author

ksunden commented Aug 3, 2022

So, while I do want input from those mentioned, I may choose to delay addressing input until a followup PR because "extant but imperfect is better than nonexistent" when it comes to docs like this.

Smaller comments and especially correctness comments I will address directly in here.

@ksunden
Copy link
Member Author

ksunden commented Aug 3, 2022

The new user reviews are primarily to gauge the approachability and understandablity with limited prior knowledge. The advanced users will likely fill in the gaps without fully realizing (like I may have done in writing them, to be honest).

I don't fully expect that you will fully understand everything immediately. Some of that will come only with usage, and I think that is fine.

It's more about laying a foundation, and providing a resource to refer back to when questions come up while using the library.

@ksunden ksunden changed the title docs fill Addition of narrative docs instead of solely API docs Aug 3, 2022
Copy link
Contributor

@ddkohler ddkohler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm real happy with these docs--they should be a long way to helping us as they exist now. Thanks @ksunden

docs/store.rst Outdated

:meth:`attune.undo` provides the instrument from prior to the latest transition.
If the transitions have occurred in memory (i.e. not stored to the Attune Store) then it simply provides the previous instrument object directly.
If instead the Instrument was loaded, it retrieves the instrument from 1 millisecond prior to the current instrument (the resolution of the Attune Store) and loads it from disk.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not getting what the diff is between in memory vs. loaded. The point about 1ms resolution reads confusingly (at first I thought you meant it undo loads one ms from current time, not the time the instrument was made). If I'm reading this correctly, there is no effective difference for the user?

Also, can you quickly note what sequential undos result in? If I undo an undo, I keep going backwards in version, correct?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the millisecond precision bit, but left in the in memory vs reaching into the store.

I'm not fully sure how to say it more clearly, at this time though...

There is to some extent a difference, though perhaps it is more minor than I think it is.

but if I create a new instrument object, then apply a series of transitions to it without storing, I can only go back to that created one.

If I instead store the created one over some other attune instrument then load it from the store, I can keep going back into the store's history.

The store history is purely linear, but if I load from an old instrument object, I can create branches in the history in memory. once I store those, it becomes linear again (a restore is used to place the loaded one at the top, then each transition is stored ultimately including the one that is desired to be the canonical one.

Remove millisecond precision, that is an implementation detail

Place retrieve before catalog
@ksunden ksunden merged commit ed41534 into wright-group:master Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants