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

Complete unit tests for BokehJS mathematical array functions #12790

Closed
ianthomas23 opened this issue Feb 7, 2023 · 3 comments · Fixed by #12892
Closed

Complete unit tests for BokehJS mathematical array functions #12790

ianthomas23 opened this issue Feb 7, 2023 · 3 comments · Fixed by #12892

Comments

@ianthomas23
Copy link
Member

We have a couple of BokehJS modules that contain mathematical array functions similar to equivalent-named NumPy functions:

  • bokehjs/src/lib/core/util/array.ts
  • bokehjs/src/lib/core/util/arrayable.ts

I recently found and fixed a couple of bugs relating to edge-case use of some of these functions in PRs #12785 and #12789. It would be good to add unit tests for all of these functions.

@DerTimonius
Copy link
Contributor

Hi @ianthomas23! I'd like to tackle this issue 🙂
(I really fancy this project and want to keep contributing in some way)

@ianthomas23
Copy link
Member Author

Hi @ianthomas23! I'd like to tackle this issue 🙂 (I really fancy this project and want to keep contributing in some way)

Please go ahead!

In case it helps, there is a unit test file for each of the source code files, in bokehjs/test/unit/core/util/array.ts and arrayable.ts. The naming is different in each file, you could just match the existing naming scheme in each file or make them similar if you prefer. Info on running the BokehJS unit tests is at https://docs.bokeh.org/en/latest/docs/dev_guide/testing.html#select-specific-bokehjs-tests and should be no problem once you have a dev environment set up, which I think you already must have.

For the new tests I was thinking of some "normal usage" tests to check they do what they are supposed to do, then the really important tests are the edge case ones. For example, do the functions accept arrays of length 0, do they accept NaNs, if they accept multiple arrays do they have to be the same length, and so on.

I don't know how much work this is. It is is easy (!) it could all be in one PR, but probably one PR per file is more sensible. Almost inevitably you will find some bugs. When you do, you can fix them if they are easy and the fixes don't break anything else, but if the fixes aren't obvious then it would be fine to leave the failing tests in the code but commented out, and we can discuss those and come back to fix them in subsequent PRs.

There is a slack channel that some of us frequent and you are welcome to join if you like communicating that way. Info is here: https://docs.bokeh.org/en/latest/docs/dev_guide.html#additional-resources

Otherwise, good luck and shout out if you need any help or guidance.

@ianthomas23
Copy link
Member Author

I forgot to say that we don't usually assign issues to people, but some of us "self-assign" to let others know we are working on an issue so that nobody else spends time duplicating work. Feel free to self-assign if you wish.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants