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

feat: Add sequences defined by a formula in n to the main tool #31

Merged
merged 1 commit into from
Oct 22, 2021

Conversation

gwhitney
Copy link
Collaborator

This is done by creating and exporting a SequenceFormula class.

In addition, adding this class seemed to spark an additional cascade
of the Vue components needing to be (more) strongly typed. So the commit
has numerous new type annotations (that should have no effect other than
to aid TypeScript type checking) and a slight modification/simplification
to the interface between sequences.ts and visualizers.ts on the one hand
and ToolMain.vue on the other, since I could not figure out out to type
the old interface (and a lot of extraneous information that was never
being used was being passed anyway). Hopefully these changes will
eliminate issue #28.

Also, again as part of shoring up the typing, finally eliminates the
spurious additional copy of the p5 code in src/assets, instead relying
on npm install to get the proper copy of p5 in the right place. (The point
being that the idiosyncratic copy was not typed, but npm brings in the
type definitions automatically if you rely on it.)

Resolves #14.

  This is done by creating and exporting a SequenceFormula class.

  In addition, adding this class seemed to spark an additional cascade
  of the Vue components needing to be (more) strongly typed. So the commit
  has numerous new type annotations (that should have no effect other than
  to aid TypeScript type checking) and a slight modification/simplification
  to the interface between sequences.ts and visualizers.ts on the one hand
  and ToolMain.vue on the other, since I could not figure out out to type
  the old interface (and a lot of extraneous information that was never
  being used was being passed anyway). Hopefully these changes will
  eliminate issue numberscope#28.

  Also, again as part of shoring up the typing, finally eliminates the
  spurious additional copy of the p5 code in src/assets, instead relying
  on npm install to get the proper copy of p5 in the right place. (The point
  being that the idiosyncratic copy was not typed, but npm brings in the
  type definitions automatically if you rely on it.)

  Resolves numberscope#14.
Copy link
Member

@liammulh liammulh left a comment

Choose a reason for hiding this comment

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

If we weren't about to add a code formatter, I would have made some comments about indentation and style. Otherwise, looks good to me.

@liammulh liammulh merged commit 2d210ff into numberscope:main Oct 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add SequenceFormula to allow specifying a sequence by a formula
2 participants