Skip to content

Commit

Permalink
Merge pull request #343 from UC-Davis-molecular-computing/dev
Browse files Browse the repository at this point in the history
Dev
  • Loading branch information
dave-doty authored Jul 4, 2020
2 parents e60b1e7 + 287341d commit 0de83c0
Show file tree
Hide file tree
Showing 21 changed files with 467 additions and 296 deletions.
51 changes: 51 additions & 0 deletions .github/workflows/gh-pages-dev.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
name: github pages dev

on:
push:
branches:
- dev

jobs:
deploy:
runs-on: ubuntu-18.04
steps:
- uses: actions/checkout@v2
with:
ref: dev

- uses: actions/checkout@v2
with:
ref: gh-pages
path: gh-pages-repo



- name: Setup Dart SDK Step 1
run: sudo apt-get update
- name: Setup Dart SDK Step 2
run: sudo apt-get install apt-transport-https
- name: Setup Dart SDK Step 3
run: sudo sh -c 'wget -qO- https://dl-ssl.google.com/linux/linux_signing_key.pub | apt-key add -'
- name: Setup Dart SDK Step 4
run: sudo sh -c 'wget -qO- https://storage.googleapis.com/download.dartlang.org/linux/debian/dart_stable.list > /etc/apt/sources.list.d/dart_stable.list'
- name: Setup Dart SDK Step 5
run: sudo apt-get update
- name: Setup Dart SDK Step 6
run: sudo apt-get -y install dart

- name: Install dependencies
run: PATH="$PATH:/usr/lib/dart/bin" pub get

- name: Install webdev
run: PATH="$PATH:/usr/lib/dart/bin" pub global activate webdev


- name: Build into gh-pages repo
run: PATH="$PATH:/usr/lib/dart/bin" pub global run webdev build -o web:gh-pages-repo/dev


- name: Deploy
uses: peaceiris/actions-gh-pages@v3
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
publish_dir: gh-pages-repo
16 changes: 12 additions & 4 deletions .github/workflows/gh-pages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,13 @@ jobs:
steps:
- uses: actions/checkout@v2

- uses: actions/checkout@v2
with:
ref: gh-pages
path: gh-pages-repo



- name: Setup Dart SDK Step 1
run: sudo apt-get update
- name: Setup Dart SDK Step 2
Expand All @@ -23,18 +30,19 @@ jobs:
run: sudo apt-get update
- name: Setup Dart SDK Step 6
run: sudo apt-get -y install dart
- uses: actions/checkout@v1
- name: Install dependencies
run: PATH="$PATH:/usr/lib/dart/bin" pub get

- name: Install webdev
run: PATH="$PATH:/usr/lib/dart/bin" pub global activate webdev

- name: Build
run: PATH="$PATH:/usr/lib/dart/bin" pub global run webdev build

- name: Build into gh-pages repo
run: PATH="$PATH:/usr/lib/dart/bin" pub global run webdev build -o web:gh-pages-repo


- name: Deploy
uses: peaceiris/actions-gh-pages@v3
with:
github_token: ${{ secrets.GITHUB_TOKEN }}
publish_dir: ./build
publish_dir: gh-pages-repo
29 changes: 25 additions & 4 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -296,24 +296,45 @@ For many typical features one would want to add that involve changing some aspec

Since DesignMain is a connected component, at the top of the file we can see how `DesignMainStrandsProps.modification_font_size` is set to equal `state.ui_state.modification_font_size` from the State. From there it propagates down to the component that needs it.

The React-Redux bindings suggest using many connected components, putting them farther down in the View tree, to avoid the need to pass so many props through intermediate React components that don't need them. For example, note that none of `DesignMain`, `DesignMainStrands`, `DesignMainStrand`, or `DesignMainStrandModifications` need the font size; they only have it for the purpose of getting it to `DesignMainStrandModificationDomain`, which needs it directly. However, in our experience, the way OverReact and OverReactRedux and currently implemented, and the way built_value is currently implemented, this was *much* slower and caused excessive jank with frequent state updates. (See [here](https://github.com/Workiva/over_react/issues/434) for more details.) It is much faster in scadnano to have only a few connected components near the top of the View tree, and to pass properties down through the view tree, even though this is annoying and requires modifying every component between the relevant component and its connected ancestor.
The React-Redux bindings suggest using many connected components, putting them farther down in the View tree, to avoid the need to pass so many props through intermediate React components that don't need them. For example, note that none of `DesignMain`, `DesignMainStrands`, `DesignMainStrand`, or `DesignMainStrandModifications` need the font size; they only have it for the purpose of getting it from the connected `DesignMain` (which gets it directly from the state through the function `mapStateToProps`) down to `DesignMainStrandModificationDomain`, which needs it directly.

However, in our experience, the way OverReact and OverReactRedux are currently implemented, and the way built_value is currently implemented, this was *much* slower and caused excessive jank with frequent state updates. (See [here](https://github.com/Workiva/over_react/issues/434) for more details.) It is much faster in scadnano to have only a few connected components near the top of the View tree, and to pass properties down through the view tree, even though this is annoying and requires modifying every component between the relevant component and its connected ancestor.

TODO: add link to a more detailed tutorial walking through the steps above showing actual code that gets added at each step.


## Pushing to the repository and documenting changes

All local commits should be push to the `dev` branch. More advanced features, for example something that takes days to implement, and might involve potentially breaking other features, should be pushed to a separate branch specific to that feature, which is then merged into `dev`. Make sure you pull changes from the repository and resolve any conflicts before pushing to the `dev` branch.
Minor changes, such as updating README, adding example files, etc., can be committed directly to the `dev` branch.

Pull requests (abbreviated PR) can be made from `dev` to `master`, but make sure that `dev` is working before merging to `master` as all changes to `master` are automatically built and deployed to https://scadnano.org.
For any more significant change that is made (e.g., closing an issue, adding a new feature), follow these steps:

1. If there is not already a GitHub issue describing the desired change, make one. Make sure that its title is a self-contained description. For example, *"problem with loading gridless design"* is a bad title. A better title is *"loading gridless design with negative x coordinates throws exception"*.

2. Make a new branch specifically for the issue. Base this branch off of `dev` (in GitHub desktop, the default is to base it off of `master`, so switch that). The title of the issue (with appropriate hyphenation) is a good name for the branch. (In GitHub Desktop, if you paste the title of the issue, it automatically adds the hyphens.)

3. If it is about fixing a bug, add tests to reproduce the bug before working on fixing it.


4. If it is about implementing a feature, add tests to test the feature.

5. Work entirely in that branch to fix the issue.

6. Run unit tests and ensure they pass.

7. Create a pull request (PR) to merge the changes from the new branch into `dev`.

Less frequently, pull requests (abbreviated PR) can be made from `dev` to `master`, but make sure that `dev` is working before merging to `master` as all changes to `master` are automatically built and deployed to https://scadnano.org.

We have an automated release system (through a GitHub action) that automatically creates release notes.

Although the GitHub web interface abbreviates long commit messages, the full commit message is included for each commit in a PR.

However, commit descriptions are not shown in the release notes. In GitHub desktop these are two separate fields; on the command line they appear to be indicated by two separate usages of the `-m` flag: https://stackoverflow.com/questions/16122234/how-to-commit-a-change-with-both-message-and-description-from-the-command-li.

So make sure that everything people should see in the automatically generated release notes is included in the commit message. GitHub lets you [automatically close](https://docs.github.com/en/enterprise/2.16/user/github/managing-your-work-on-github/closing-issues-using-keywords) an issue by putting a phrase such as "closes #14". Although the release notes will link to the issue that was closed, they [will not describe it in any other way](https://github.com/marvinpinto/actions/issues/34). So it is important, for the sake of having readable release notes, to describe briefly the issue that was closed in the commit message. (One simple way to do this is to copy/paste the title of the issue into the commit message.)
So make sure that everything people should see in the automatically generated release notes is included in the commit message. GitHub lets you [automatically close](https://docs.github.com/en/enterprise/2.16/user/github/managing-your-work-on-github/closing-issues-using-keywords) an issue by putting a phrase such as "closes #14". Although the release notes will link to the issue that was closed, they [will not describe it in any other way](https://github.com/marvinpinto/actions/issues/34). So it is important, for the sake of having readable release notes, to describe briefly the issue that was closed in the commit message.

One simple way to do this is to copy/paste the title of the issue into the commit message. For this reason, issue titles should be stated in terms of what change should happen to handle an issue. For example, instead of the title being *"helices are displayed at the wrong y-coordinate in the honeycomb grid"*, a better issue title is *"display helices at the proper y-coordinate in the honeycomb grid"*. That way, when the issue is fixed in a commit, that title can simply be copied and pasted as the description of what was done for the commit message. (But you should still add "fixes #<issue_number>" in the commit message.)

Users can read the description by clicking on the link to the commit or the pull request, but anything is put there, then the commit message should say something like "click on commit/PR for more details".

Expand Down
21 changes: 19 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ If you find scadnano useful in a scientific project, please cite its associated

> <ins>scadnano: A browser-based, scriptable tool for designing DNA nanostructures</ins>.
David Doty, Benjamin L Lee, and Tristan Stérin.
DNA 2020: *Proceedings of the 26th International Conference on DNA Computing and Molecular Programming*
DNA 2020: *Proceedings of the 26th International Conference on DNA Computing and Molecular Programming*
[ [paper](https://arxiv.org/abs/2005.11841) | [BibTeX](https://web.cs.ucdavis.edu/~doty/papers/scadnano.bib) ]

## Table of contents
Expand Down Expand Up @@ -61,7 +61,9 @@ Please file bug reports and make feature requests as GitHub repository issues in
or the
[Python scripting library](https://github.com/UC-Davis-molecular-computing/scadnano-python-package/issues).

Early versions of this project didn't have well-defined versions. However, we will try to announce breaking changes (and possibly new features) under the [GitHub releases page](https://github.com/UC-Davis-molecular-computing/scadnano/releases). The version numbers in this web interface repo and the [Python library repo](https://github.com/UC-Davis-molecular-computing/scadnano-python-package/releases) won't always advance at the same time. However, when a breaking change is made, this will increment the minor or major version numbers in both libraries (version numbers are major.minor.patch, i.e., version 0.9.2 has minor version number 9).
Early versions of this project didn't have well-defined versions. However, we will try to announce breaking changes (and possibly new features) under the [GitHub releases page](https://github.com/UC-Davis-molecular-computing/scadnano/releases). The version numbers in this web interface repo and the [Python library repo](https://github.com/UC-Davis-molecular-computing/scadnano-python-package/releases) won't always advance at the same time.

Following [semantic versioning](https://semver.org/), version numbers are major.minor.patch, i.e., version 0.9.2 has minor version number 9. Prior to version 1.0.0, when a breaking change is made, this will increment the minor version (for example, going from 0.9.4 to 0.10.0). After version 1.0.0, breaking changes will increment the major version.


## Reporting issues
Expand Down Expand Up @@ -97,6 +99,21 @@ To disable this so that it uses the same filename every time you save, you can i



## Stable and development versions

- stable: https://scadnano.org
- dev: https://scadnano.org/dev

The scadnano stable version matches what is on the [master branch of the web interface code repository](https://github.com/UC-Davis-molecular-computing/scadnano).
The scadnano dev version matches what is on the [dev branch of the web interface code repository](https://github.com/UC-Davis-molecular-computing/scadnano/tree/dev).


Releases of the stable version are explained on the [releases page](https://github.com/UC-Davis-molecular-computing/scadnano/releases).
When [issues](https://github.com/UC-Davis-molecular-computing/scadnano/issues) are handled in a release, they are closed at the time the changes make their way to the master branch.
If an issue is handled in the dev branch, the issue remains open, but you will see a comment that looks something like this:
"*dave-doty added a commit that referenced this issue 17 hours ago @dave-doty make width of File menu just enough to fit all entries on one line; fixes #339*". These comments can help you decide if you want to use the latest version of scadnano (https://scadnano.org/dev), which has fixed an issue, before it makes its way to the stable version (https://scadnano.org).




## Terminology
Expand Down
6 changes: 6 additions & 0 deletions fix-index.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# GitHub Desktop sometimes cannot write the .git/index file.
# Copying it, deleting the original, and moving the copy back fixes it.
cp .git/index index-copy
rm .git/index
mv index-copy .git/index

2 changes: 1 addition & 1 deletion lib/src/constants.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import 'package:platform_detect/platform_detect.dart';

import 'state/grid.dart';

const String CURRENT_VERSION = "0.9.4";
const String CURRENT_VERSION = "0.9.6";
const String INITIAL_VERSION = "0.1.0";

const BUG_REPORT_URL = 'https://github.com/UC-Davis-molecular-computing/scadnano/issues';
Expand Down
11 changes: 6 additions & 5 deletions lib/src/reducers/dna_design_reducer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ DNADesign dna_design_reducer(DNADesign dna_design, action) {
return dna_design;
}

// This isn't strictly necessary, but it would be nice for debugging if, whenever there is an error,
// the DNADesign in the Model is null.
//DNADesign error_message_set_reducer(DNADesign dna_design, actions.ErrorMessageSet action) =>
// action.error_message == null || action.error_message.length == 0 ? dna_design : null;

DNADesign dna_design_global_reducer(DNADesign dna_design, AppState state, action) {
dna_design = dna_design_composed_global_reducer(dna_design, state, action);
dna_design = dna_design_whole_global_reducer(dna_design, state, action);
Expand Down Expand Up @@ -53,9 +48,15 @@ DNADesign dna_design_composed_global_reducer(DNADesign dna_design, AppState stat
// whole: operate on the whole DNADesign
// local: don't need the whole AppState
Reducer<DNADesign> dna_design_whole_local_reducer = combineReducers([
TypedReducer<DNADesign, actions.ErrorMessageSet>(dna_design_error_message_set_reducer),
TypedReducer<DNADesign, actions.InlineInsertionsDeletions>(inline_insertions_deletions_reducer),
]);

// This isn't strictly necessary, but it would be nice for debugging if, whenever there is an error,
// the DNADesign in the Model is null.
DNADesign dna_design_error_message_set_reducer(DNADesign dna_design, actions.ErrorMessageSet action) =>
action.error_message == null || action.error_message.length == 0 ? dna_design : null;

// whole: operate on the whole DNADesign
// global: need the whole AppState
GlobalReducer<DNADesign, AppState> dna_design_whole_global_reducer = combineGlobalReducers([
Expand Down
4 changes: 2 additions & 2 deletions lib/src/reducers/helices_reducer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,7 @@ DNADesign helix_add_dna_design_reducer(DNADesign design, AppState state, actions
Helix helix = Helix(
idx: new_idx,
grid: design.grid,
geometry: design.geometry,
grid_position: action.grid_position,
position: action.position,
min_offset: min_offset,
Expand All @@ -177,9 +178,8 @@ DNADesign helix_add_dna_design_reducer(DNADesign design, AppState state, actions
);
Map<int, Helix> helices = design.helices.toMap();
helices[helix.idx] = helix;
var geometry = design.geometry;
bool invert_y_axis = state.ui_state.invert_y_axis;
helices = util.helices_assign_svg(geometry, invert_y_axis, helices, design.grid);
helices = util.helices_assign_svg(design.geometry, invert_y_axis, helices, design.grid);

return design.rebuild((d) => d..helices.replace(helices));
}
Expand Down
1 change: 1 addition & 0 deletions lib/src/state/dna_design.dart
Original file line number Diff line number Diff line change
Expand Up @@ -610,6 +610,7 @@ abstract class DNADesign with UnusedFields implements Built<DNADesign, DNADesign
}
helix_builder.invert_y_axis = invert_y_axis;
helix_builder.grid = dna_design_builder.grid;
helix_builder.geometry = geometry.toBuilder();
if (grid_is_none && helix_json.containsKey(constants.grid_position_key)) {
throw IllegalDNADesignError(
'grid is none, but Helix $idx has grid_position = ${helix_json[constants.grid_position_key]}');
Expand Down
2 changes: 1 addition & 1 deletion lib/src/state/geometry.dart
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ abstract class Geometry with BuiltJsonSerializable, UnusedFields implements Buil

/************************ end BuiltValue boilerplate ************************/

/// What is this?
/// Distance in nanometers between two adjacent base pairs along the length of a DNA double helix.
double get rise_per_base_pair;

/// Radius of a DNA helix in nanometers.
Expand Down
9 changes: 7 additions & 2 deletions lib/src/state/helix.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import 'package:scadnano/src/state/unused_fields.dart';

import '../json_serializable.dart';
import '../serializers.dart';
import 'geometry.dart';
import 'grid.dart';
import 'strand.dart';
import 'grid_position.dart';
Expand Down Expand Up @@ -58,6 +59,7 @@ abstract class Helix with BuiltJsonSerializable, UnusedFields implements Built<H
factory Helix({
int idx,
Grid grid,
Geometry geometry,
int view_order = null,
GridPosition grid_position = null,
num roll = constants.default_helix_roll,
Expand All @@ -75,6 +77,7 @@ abstract class Helix with BuiltJsonSerializable, UnusedFields implements Built<H
return Helix.from((b) => b
..idx = idx
..view_order = view_order
..geometry = geometry?.toBuilder()
..grid = grid
..grid_position = grid_position?.toBuilder()
..position_ = position?.toBuilder()
Expand All @@ -98,6 +101,8 @@ abstract class Helix with BuiltJsonSerializable, UnusedFields implements Built<H

Grid get grid;

Geometry get geometry;

/// position within square/hex/honeycomb integer grid (side view)
@nullable
GridPosition get grid_position;
Expand Down Expand Up @@ -147,7 +152,7 @@ abstract class Helix with BuiltJsonSerializable, UnusedFields implements Built<H

@memoized
Position3D get default_position {
num x = min_offset * constants.BASE_WIDTH_SVG;
num x = min_offset * geometry.rise_per_base_pair;
Point<num> svg_pos = util.side_view_grid_to_svg(grid_position, grid, invert_y_axis);
Position3D position3d = util.svg_side_view_to_position3d(svg_pos, invert_y_axis).rebuild((b) => b..x = x);
return position3d;
Expand Down Expand Up @@ -239,7 +244,7 @@ abstract class Helix with BuiltJsonSerializable, UnusedFields implements Built<H
/// given helix idx and offset, depending on whether strand is going forward or not.
/// This is relative to the starting point of the Helix.
Point<num> svg_base_pos(int offset, bool forward) {
num x = constants.BASE_WIDTH_SVG / 2 + offset * constants.BASE_WIDTH_SVG + this.svg_position.x;
num x = constants.BASE_WIDTH_SVG / 2.0 + offset * constants.BASE_WIDTH_SVG; // + this.svg_position.x;
// svg_height is height of whole helix, including both forward and reverse strand
// must divide by 2 to get height of one strand, then divide by 2 again to go halfway into square
num y = svg_height() / 4.0 + this.svg_position.y;
Expand Down
Loading

0 comments on commit 0de83c0

Please sign in to comment.