-
Notifications
You must be signed in to change notification settings - Fork 0
Update RTD to support new users #249
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
Conversation
- Put Fiji in the quick start section since this is how we expect people to interact with Ops - Focus the how-to guides to cover usage in-depth - Move benchmarks to Development since because of the technical scope of this section
Add some info on creating an OpEnvironment and add more words to clarify API usage.
- Expand to cover Function and InPlace options - Reword for clarity
ctrueden
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hinerm This is super great! Much friendlier! 🤩
I pushed some commits with minor adjustments on top of your new prose, and I think we're good to merge!
| Note that although the final method call changes for each mode of operation, *this is based on the path taken through the `OpBuilder` chain*. For example, we cannot call the `compute()` method if we haven't provided an `.output()`: | ||
|
|
||
| ``` | ||
| # Does not compute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😂
| ```text | ||
| filter.gauss: | ||
| - (input, sigmas, @CONTAINER container1) -> None | ||
| - (input, sigmas, outOfBounds = null, @CONTAINER container1) -> None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not part of this PR I know, but this output mixes nulls and Nones... we should make that more consistent.
gselzer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that @ctrueden merged before I got the chance to finish my review, but we can still think about my points if we want! None are really important!
| With the name established in the `OpBuilder` chain, we can then specify our input(s) with the `.input()` method. | ||
|
|
||
| For our gaussian blur, we will pass as inputs our input image `inImage`, and a `double` as our sigma parameter: | ||
| For this Gaussian blur, we have two inputs: `inImage` is the image we want to blur, and a `double` value as our sigma parameter: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| For this Gaussian blur, we have two inputs: `inImage` is the image we want to blur, and a `double` value as our sigma parameter: | |
| For this Gaussian blur, we have two inputs: `inImage` is the image we want to blur, and a `double` value as the standard deviation of the smoothing function: |
What do you think about using the technical term?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the simpler our sigma parameter 😉
| var outImage = ops.op("filter.gauss").input(inImage, 2.0).apply() | ||
| ``` | ||
|
|
||
| *Inplaces* are used when we want to destructively modify one of the existing inputs (which is explicitly forbidden by *computers*; a *computer* Op's output should be a different object from all of its inputs). We indicate this by the `mutate#()` method, where the `#` corresponds to the *parameter index* that will be modified: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be confusing, or clarifying, to note this is 1-indexed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| *Note that the default `OpEnvironment` implementations cache Op requests* - this means that repeated `OpBuilder` requests targeting the same action will be faster than the original matching call. | ||
|
|
||
| ## Additions: Matching with classes | ||
| ### Matching with classes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is classes better or worse than types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make further updates based on feedback from PR #249
These changes restructure the introductory sections of the read the docs pages to get users into Ops use relatively smoothly (hopefully) by tuning the
Getting Startedsection to be more of a "quick start" into scripting, while focusing and fleshing out the "how-to guides" section, e.g. by prioritizing finding Ops before calling them and adding missing information onFunctionsandInplaces.