spacetelescope / style-guides Goto Github PK

It is unclear to me what the thingies in "templates" are used for at a glance. Are they replacing what is in STScI package template? A README in that dir to clarify things would be nice.

I am no lawyer, so I don't understand why CC-BY-4.0 here and not BSD. Thanks!

Add a page on best practices for security.

I am not sure whether this should be an issue here (style-guides) or rather on the stsci-package-template:

I followed the steps described at https://github.com/spacetelescope/style-guides/blob/master/guides/python-package.md for creating a new package/repo (https://github.com/spacetelescope/pystortion)

The travis build fails (https://travis-ci.com/spacetelescope/pystortion/builds/91425159) with ModuleNotFoundError: No module named 'astropy_helpers'

Did I make a mistake in the process or is there an oversight in the guide or template?

@eteq: We should say something about classes vs functions. There's not an obvious answer of course, but I think something that gets across the idea that "you should learn enough about the general concepts of OOP to know when it might be called for" is warranted. @SaOgaz has some good works in the STAK notebooks about this.

Create a page for instructions for code review

The versioning guide is great and contains a lot of detail. In the future, we may want to revisit it to take a look at having it focus more directly on the recommendations and saving some of the detail (especially the python 2.x which likely can be depreciated as we are recommending Python 3 only support -- but it is currently useful!)

Explain how to create a good notebook. Should probably cover:

• Documentation
• Structure
• APIs vs hard-coded paths
• Include a requirements.txt to describe dependencies?

• Clean up the README
• Explain difference between original template branch

Include details for how to include notebooks in the automatically built documentation

Two pieces of advice for avoiding later headaches:

• google the repository (and package name, if different).
  • If it already exists consider choosing a different name
  • if the repository acronym can have offensive meanings in other contexts try something different

@eteq:

We should probably mention something about wrapping C code. I don't think we want to be overly-perscriptive, but maybe something like "Try Cython because it's easier for others to read, supports numpy (use the memoryview technique for that) and builds with a python project mostly transparently. If that doesn't work, try other reasonably standard tools like pybind. If that doesn't work, use the C-API directly but use existing build machinery instead of rolling your own if at all possible."?

😉

There is some debate on astropy-helpers and whether or not to base our packages on it -- see the discussion here: spacetelescope/stsci-package-template#16 . As that is an intermediate issue, opening this ticket here for further discussion about it.

Related to #12, but we do need a page with details about how to do a release so that it is easily included in astroconda

A new python package was being established for supporting HST pipeline work and, in following the current style guide, this new package was initially set up with the name hla-pipeline. Subsequent use of 'cookiecutter' to initialize the repo, it returned with the following error:

> ERROR: The project slug (hla-pipeline) is not a valid Python module name. Please do not use a - and use _ instead
> ERROR: Stopping generation because pre_gen_project hook script didn't exit successfully
> Hook script failed (exit status: 1)
> 

Therefore, it appears that the guide needs to be updated to reflect the actual Python rules for package names and not allow '-' (hyphens) in the names.

Create a guide with the best practices for git workflow.

Currently, two of the three non-master branches are from merged PRs. Any reason to keep them?

Issues should be milestoned for v1.0 to determine what needs to be delivered. Please open up an additional issues for things that are needed.

Not everyone is currently in the space telescope organization and on github, which means that some people might not be able to view this repository. Does this now have sufficient content and enough of a start to make public?

Something brought up by @samanthalh -- When one is developing notebook for users and find oneself in a situation where one needs to copy and paste the same code with slight variations (e.g., plotting results of subsequent tweakreg runs on the same dataset), is it better to:

1. Brute force copy and paste; OR
2. Write a function of the repeated code and then call it with different inputs where they vary?

Brute force way is more tedious and provides clarity and possibly is resistant to out-of-order cell runs in some cases. A function might be more elegant and reduce future maintenance but user needs to scroll up to see what happens in the function, and the cell with the function needs to be run before all the other stuff.

Is a guide line for such a thing within scope of this document?

In #38 (comment), @pllim (+ @stscicrawford) pointed out that having the Code of Conduct be a link rather than full text might make life easier.

Generally I like this idea, but two questions about this:

1. Where would we put the "canonical" copy? Somewhere on github might seem logical - maybe even the style guide repo itself? But it could also be somewhere on the STScI web site.
2. Are there subtle legalities or something we should be worried about if we don't include the text a la how it might work with licenses? (I think not - as @pllim pointed out plenty of other projects like Astropy use this - but worth asking at least...)

@stscicrawford said I might need to open PRs to the repos to apply the changes? Or maybe we link them back here? Please advise.

This came up at the drizzlepac notebooks hack day.

We should agree on whether notebooks should be:

1. notebook_directory
2. notebook-directory
3. notebookDirectory
4. notebook directory.

I vote for 1, am ok with 2, and dislike 3 and 4. Any opinions?

cc @shannnnnyyy (who first asked about it)

Maybe this is not feasible as the contribution process can differ project by project. I was thinking something like http://docs.astropy.org/en/latest/development/workflow/development_workflow.html but more attuned to spacetelescope org. The goal is to provide someone a guideline in contributing to a project that they are not familiar with. This would go well with, say, a PR review guideline that @brechmos-stsci will open a PR for shortly.

Example (and fictional) use case: A STScI instrument scientist who found a bug in stsci.tools, figured out a patch, and wish to contribute for the first time.

Currently, https://github.com/spacetelescope/stsci-package-template/blob/master/CONTRIBUTING.md simply asks the interested party to contact package maintainers.

This may belong better in the "Package Conventions" section, but it would probably be useful to say a bit about using the from . import * trick as a way to internally structure code, since that's an exception to the import * rule as long as you're careful about what goes in all.

In https://innerspace.stsci.edu/display/DATB/Python+Version+Support+Plan+for+DATB I wrote up a document about practices for supporting versions of Python and external dependencies in DATB. We should have something like that here (happy to just translate my words into this repo if that makes sense). A more general policy exists (but perhaps is not ratified?) for ST here: https://innerspace.stsci.edu/display/isec/Software+Language+Supported+Versions

possibly in the same document we should describe more about what versions of our software we will support. By its nature this would be intertwined with #12, but there's probably a separable "how should I version my software" from the details of "what workflow supports that versioning scheme".

It seems to me that not all guides under "guides" are listed in top-level README. Is this intentional?

Guidelines for setting up jenkins and travis and openCI

It would be useful to have a page with tips on accessibility and ways to improve code, images and documentation to make it easier and more accessible for people

"An opinionated guide to setting up STScI-owned GitHub repositories maintained by @spacetelescope/style-gurus "

Inspired by https://github.com/astropy/package-template/

p.s. I am talking about possibly removing CODEOWNERS file.

I am interested in this one if you have something that is proven to work and you can recommend as part of style guide. Think of it as flake8 check that is built into the editor, but for notebooks.

One practical question that comes to mind - is this the right place to describe tools and procedures for configuring an environment to comply with these guides? Perhaps have a section at the end that links to existing descriptions of how to configure IDEs or editors to check for PEP8 compliance?

In the code of conduct, in the last bullet, is this sentence, which is confusing partway through. Bold added:

We will take action when members of our community violate this code such as such as contacting [email protected] (all emails sent to this address will be treated with the strictest confidence) or talking privately with the person.

First, "such as such as" is clearly an accidental repetition. But more problematically, as written this seems to suggest that "contacting conduct@stsci" is itself an example of a possible code violation that we will take action about...

This error applies to both copies of CODE_OF_CONDUCT.md in this repo, and presumably others elsewhere.

It's great that we've got a GitHub guide in here (✨ @bourque), however, I think the fork/PR workflow we're recommending is the wrong one for repositories under the spacetelescope organization.

Reasons I don't like forks:

• Forks are harder to back up: ITSD keeps backups of our Git(Hub) repositories. This is a contractual requirement. If we're all working in our own personal forks, then ITSD has to now back up all of the forks as well as the repositories under the spacetelescope organization.
• Forks are a usability nightmare:
  • They require extra work to keep up to date with the parent (git fetch upstream etc).
  • If forks are of a public repository and more than one person wants to collaborate on a fork then you have to add each colleague to your fork
  • If forks are of a private repository then everyone watching the parent repository is automatically subscribed to all of the fork activity by default (GitHub notification-hell)
• CI isn't enabled by default on forks - i.e. if the parent repository has Travis-CI turned on, then you have to open a pull request before Travis will start running tests. When pushing feature branches, CI can easily be enabled for all branches.

Instead of what we currently have in the GitHub workflow document, we propose the following:

When STScI staff are working on an STScI-owned project, staff should push feature branches to a shared repository rather than working on personal forks

Reasons I often hear against a feature-branch-on-a-shared-repository proposal:

• Equality of experience: If we have STScI staff pushing feature branches and non-STScI staff having to fork an open source repository then they have a different user experience.
• People like to work privately: Forks are good because they are 'private'. IMO within an organization, this isn't a great reasons. I think we want more transparency in the workplace.
• I don't like cluttering up the main repository with lots of branches...
• What if people overwrite master? GitHub's branch protection should help us out here.

/ cc @eteq @bourque @stscicrawford (i.e. folks I've already discussed this with at least in part)

What if someone accidentally committed something undesirable? Who should the person contact or how can the person undo the commit and remove it from history? Might be worth documenting?

Asking for a friend.

For someone coming in fresh to the style guide, it would be useful to add some top level instructions and an introduction to the style guide.

In the jupyter style guide, it would be useful to suggest other ways to request files.

It would be nice to have a guide that provides institute-recommended styles for web development, providing CSS and base HTML templates.

@cam72cam and @Jenneh and @bourque compared notes on our existing CSS templates, which follow closely to the new STScI website design. We believe this is a good starting point.

FWIW, attached are the web style guide and corresponding CSS template for the jwql project (created by @laurenmarietta)

jwql.css

This is related to #6 but different enough to be worth a new issue, I'd say.

The idea here (which @stscicrawford and I just talked about) would be to lay out a few different release/repo branch strategies, organized so that there are "upgrade" paths between them. The three I think make the most sense are:

1. All work happens on master, when a new version is ready, release and never support anything again. Bug fixes always just go into master. (the "scientist" use case?)
2. All work happens on master, but there is a maintenance branch that is only the most recent release. Hotfixes come out of that branch as new tags, get backported manually, with milestones (or a "needs-backporting" label?) on github used to identify which should be backported. (I think this is basically the workflow in https://innerspace.stsci.edu/display/~dencheva/Github+and+Release+Workflow )
3. The astropy workflow, where there is a latest major branch and some number (for astropy just one) of older versions to maintain. See the Astropy docs like http://docs.astropy.org/en/stable/development/releasing.html for more on this

All come with the option (probably recommended for all except maybe 1) of a stable branch that points to the latest release.

Another issue that would be useful to have a style guide for: linked data directories.

This is particularly relevant to notebooks, which might operate on a data file that they need to find in order for an end-user to run the notebook successfully. I can see two kinds of files:

1. Small data files that make sense to live within the repository itself, and can easily be linked with an environmental variable
2. Large data files that shouldn't be in the repository but staged elsewhere.

Being relatively new to git and python (from svn and IDL) I've been inventing my own approaches using environmental variables pointing to (1) the local repo checkout directory, and (2) a corresponding data directory on central store. Works well enough, but if there is a more elegant solution it would be helpful to describe here.

Heya, there's a fair amount of stuff in here that I'm not convinced are style guides. Examples would include:

• Release/maintainence workflow and repository branch structure: #12
• Git Workflow: #6
• ...others

At lot of the open tickets are what I would describe as 'workflow' guides rather than style guides. While I think defining workflows is a really good idea, it's just not obvious that they should be in this repo.

Thoughts @stscicrawford @eteq?

Create a style guide outlining best practices for writing python code and packages

Do we have a convention for team names? I guess this is slightly related to #10 and thus a team name should use a hyphen over an underscore, however, should we use a hyphen or whitespace?

In #23 I've created a first initial pass at a notebook style guide. There are a few known issues I'd like us to address:

• License statement: We should include a license statement (suggest CC-BY) at the bottom of each notebook.
• Table of contents: Currently, the example notebooks we link to have Tables of Contents created in a less-than-optimal way (lots of custom HTML). Currently JupyterLab doesn't seem to support native Markdown lists with section links - we need to find a workaround here to https://stackoverflow.com/questions/50467064/unable-to-use-anchor-tags-to-create-table-of-contents-in-jupyterlab.
• What to do about custom requirements?: We could ask people to define a requirements.txt file but currently we wouldn't use it anywhere...

https://github.com/spacetelescope/style-guides/blob/master/guides/github-repositories.md
Last line, under Status heading, "Acively" should be "Actively"

Tyler Desjardins mentions that we should consider moving emails from help[at]stsci.edu to point to the web portal where possible and appropriate. For HST (or any non-JWST), it is https://hsthelp.stsci.edu . For JWST, it is https://jwsthelp.stsci.edu . Please update info in setup.py, setup.cfg, documentation, etc as appropriate.

Please close this issue if it is irrelevant to your repository. This is an automated issue. If this is opened in error, please let pllim know!

xref spacetelescope/hstcal#317

A discussion with ITSD lead to the conclusion that the best current solution to recommend here for the is to tell internal user to put things on box and use a public link. Medium-term we'll develop a redirect tool that lets it instead be a more descriptive URL, but since the backing store will likely be Box, we can stick with that as the immediate recommendation. Need to update the "data guide" to reflect this.

cc @stscicrawford

EDIT: Also see https://jira.stsci.edu/browse/JUSI-26

Add information about testing workflows.

We should include instructions for how to create a new repository