-
Notifications
You must be signed in to change notification settings - Fork 44
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
Readme suggestions #84
Conversation
As discussed in spacetelescope#74
…esky into upstream_master Merged upstream master
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 have several wording thoughts below but generally this looks good, thanks @camipacifici.
One broader item: do we want to specify a bit more about the science/technical review process? I.e., that the results of the technical review should go in a separate notebook or something? Or should we do that as a separate follow-on PR?
3. Notebook-driven development | ||
4. Integrated Notebook | ||
5. Final/Public Notebook | ||
4. Advanced Stage |
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.
4. Advanced Stage | |
4. Advanced Integration Stage |
I know we want to follow the DAWG wording, but I'm thinking we still want something that indicates this stage is really about "having the tools and the notebooks work together" instead of just "better" (which is what "advanced" sounds like to me).
I'm not necessarily attached to this exact wording but hopefully the idea gets across here.
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.
Sounds good to me!
analysis tools, or it may only require a few minor adjustments (or none at all!). This stage is therefore the most | ||
flexible and dependent on developer resources, etc. In general the intent is for developers to be able to re-use | ||
bits of code from the notebook as tests for development, while occasionally (if necessary) asking the notebook | ||
author for guidance to ensure the implementation actually meets the notebook's needs. There is not a formal | ||
process for this step, but it is intended that the JDAT planning process (currently on JIRA) keeps track of specific | ||
steps needed before a given notebook can proceed on to the next stage. | ||
|
||
## Integrated Notebook | ||
## Advanced Stage |
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.
## Advanced Stage | |
## Advanced Integration Stage |
(or whatever is decided in my comment above)
In either case, the title (but not filename) of the notebook should begin with "Draft:" to indicate the | ||
notebook is in the draft stage. | ||
In either case, the title (but not filename) of the notebook should begin with | ||
"Baseline:" to indicate the notebook is in the Baseline Stage. |
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.
We haven't necessarily been doing this. Do we want to stick with this, or should we change to the title just being the title and categorizing it using github or Jira?
(If we do still like this we should probably add it to the technical review to make sure the title is right...)
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 would like to have the title in the notebook (not in the file) to reflect the status, so "Baseline: blablabla...". Agree, we should add this to the technical review!
but in that case it is critical that the notebook state prominently that it must be run inside the STScI network. | ||
3. A notebook should state clearly what version of various dependencies were used to generate the notebook. | ||
These versions should be placed in a `requirements` file in the same directory as the notebook itself. An example of this file | ||
is in the``example_notebook`` folder. | ||
That will ensure reviewers/testers can be sure that if they encounter problems, it is not due to software version mis-matches. | ||
|
||
The notebook will undergo a scientific and a technical review and it will be merged into the repository | ||
once the review comments have been addressed. This concludes the Baseline Stage. |
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.
once the review comments have been addressed. This concludes the Baseline Stage. | |
once the review comments have been addressed. At this point the notebook has formally reached the Baseline Stage. |
Maybe this is being overly-pedantic, but I think we want to call the notebook baseline when it's done with this process, rather than notebooks that are currently undergoing the process being called "baseline".
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.
Yes, I agree with you. The wording I used was misleading. It is clearer with your edit!
Between the concept and draft, or draft and polished stages, there is potential for considerable development | ||
to be necessary. A draft notebook may contain a large number of areas where more development is desired in data | ||
Along and after the Draft and Baseline stages, there is potential for considerable development | ||
to be necessary. A baseline notebook may contain a large number of areas where more development is desired in data |
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.
to be necessary. A baseline notebook may contain a large number of areas where more development is desired in data | |
to be necessary in the libraries or other tools that the notebook uses. A baseline notebook may contain a large number of areas where more development is desired in data |
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.
Ok
the Pull Request workflow as described above, but with the notebook title now changed to be just the title itself | ||
(no "Concept:" or Draft:"). The Pull Request is then reviewed by one of the project scientists, and merged when | ||
the Pull Request workflow as described above, but with the notebook title now changed to be just | ||
the title itself (no "Draft:" or Baseline:"). The Pull Request is then reviewed by one of the project scientists, and merged when |
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 hadn't thought about this until now, but perhaps we want to replicate the sci/tech review process from the above at this time? As it stands it's now just "the project scientist", which seems a bit of a bottleneck relative to the baseline review process.
(Of course the sci/tech review could be a project scientist, but we don't have to mandate it.)
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.
Right. Better say "by a developer and a scientist".
Thank you for catching these things!
Interesting thought. I've considered it rather clumsy to have comments to the author & developers in the same notebook that is being published for external users. Maybe this is a good use for git branches? Create a branch for the annotated versions of the notebooks for each stage of the notebooks (draft, baseline & optimized)..with the version in that branch having all the annotations that are best put in the notebook rather than the PR? It's a bit cumbersome, itself, but if we consider the versions in the branch as "annotated snapshots" of the notebook that was reviewed, we aren't committing to maintaining parallel versions. The downside might be that these annotated copies would be harder to find, but that could be an upside too since they are really just for internal use. |
There was a fair amount of discussion on the broader question above in #84 (comment), which we didn't capture here (it's in the ST Jira). But since it's above and beyond these changes, I merged this and will make an issue capturing what I think are the actions to address that. |
I changed the terminology according to the road map.