Plasmapy: Code review for v0.1

Created on 12 Jan 2018  路  7Comments  路  Source: PlasmaPy/PlasmaPy

Prior to completing the v0.1 milestone we should do a code review, as per Best Practices for Scientific Computing. We already do this to some extent via code reviews during pull requests, but it would be nice to have a look at the codebase as a whole.

This would entail sifting through all the code (including getting fresh eyes on code since many of us develop our own niches) looking for:

  • bugs
  • stylistic inconsistency
  • bad practices
  • lack of documentation or places where it could be improved
  • non-exhaustive tests
  • performance issues and potential optimizations

One way we could go about doing this is to open a code review issue for each module and assign 1 or 2 people (preferably people who haven't worked with and who are unfamiliar with the module) to that issue. Those persons will then sift through the code and open up new, minor issues. We can then have either reviewers or people more familiar with the module correct those issues, whichever is more appropriate.

Thoughts?

high Help wanted high Testing

All 7 comments

Looks like a good learning experience. I can do a module.

@namurphy When are we planning the freeze and would this be best done before or during the freeze? (e.g., freeze code, review, then unfreeze for fixes raised in review)

@lemmatum I think the term we used was "feature freeze", and that was supposed to be just about now. In other words, we'll probably try to squeeze the current PRs in before mid february. Perhaps the start of February could be a good time to do the code review?

@StanczakDominik sounds good to me. I'll try and get my PRs done in time for that.

I'd like to explicitly add to the list of things to check: consistency in vTh definitions. Sometimes plasma functions are defined with vTh that has a factor of 2 in the square root and sometimes without it. We should explicitly check that the equations we use are compatible with the sqrt(2) version as this is what we use for "most probable" thermal velocity.

We probably will have time for about one or two ~60-90 minute code review session per week, so we should prioritize. What modules are the most important ones for us to review?

In the atomic subpackage, we just reviewed the Particle class in particle_class.py this past week. I'd also like for us to review the @particle_input decorator, and possibly parsing.py. We are planning on going over physics/parameters.py next Tuesday during our usual biweekly meeting time. After that, I would say that the transport and Braginskii coefficient functionality is the highest priority.

We should leave updates and links on what we've reviewed here.

Module review checklist:

  • [x] atomic.py
  • [ ] Plasma.py
  • [ ] mathematics.py
  • [ ] dimensionless.py
  • [ ] distribution.py
  • [x] parameters.py
  • [ ] quantum.py
  • [ ] relativity.py
  • [ ] transport.py

    • [ ] collisions.py

    • [x] braginskii.py

We should also go through the respective tests and cleanup that code.

  • [ ] test_atomic.py
  • [ ] test_Plasma.py
  • [ ] test_mathematics.py
  • [ ] test_dimensionless.py
  • [ ] test_distribution.py
  • [ ] test_parameters.py
  • [ ] test_quantum.py
  • [ ] test_relativity.py
  • [ ] test_transport.py

    • [ ] test_collisions.py

    • [ ] test_braginskii.py

I think we did what we could here. The checklist may come in handy for the future, but for now I'm going to close this.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

namurphy picture namurphy  路  3Comments

namurphy picture namurphy  路  5Comments

lemmatum picture lemmatum  路  3Comments

namurphy picture namurphy  路  5Comments

lemmatum picture lemmatum  路  5Comments