-
Notifications
You must be signed in to change notification settings - Fork 648
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
increase code coverage in PSA #1006
Comments
I've been working on increasing coverage of this module. I think there are algorithm issues with some of the path code (see #1005), but I've generally been able to deal with plotting-related coverage by writing unit tests that check for return of a And the streamline visualization code I contributed does nothing but plot from an enduser standpoint, so I think it can be ok. |
Just adding context: The issued in PR #1005 is a documentation issue, not a bug as far as I can tell, but @sseyler should be able to clear any remaining doubts. The plotting in PSA is pretty complicated and it is a good thing to give users some working examples. Just test for returning a matplotlib object as @tylerjereddy said. |
we can also test the actual image. When we save a reference image it should be possible to get the matplotlib image as a bitmap that we can compare to a stored reference image. |
^ Heh. Thats kind of one of those letter of the law vs spirit of the law sort of things. |
@sseyler can you please have a look and comment? Then we can decide what to do – possibly add more tests so that the code is covered. Our guidelines say that analysis code must have at least 70% coverage or code is moved to |
Ping @sseyler |
I'll get on that starting tomorrow! (I actually talked to Taylor Colburn today about helping to update the PSA code and contributing on GitHub, as he's also found some issues and points of improvement.) |
Once 1.0 comes around, coverage of PSA needs to be up to our standards and have ≥ 70% code coverage (more is obviously better). |
Update: I gave Taylor the full overview today so he has most of what's needed to start writing tests for PSA (git, github + PRs, virtualenvs + editable installs, unit testing + pytest, etc.). Hopefully he (and I) will get some more tests done this week. |
- fix MDAnalysis#1006 - Updated docs for fit_to_reference; added doc for postfix parameter and note about prefix - Added prefix parameter to Path.run to match optional prefix parameter accepted by fit_to_reference. Updated docs for Path.run() to match Path.fit_to_reference. - Code to pass prefix parameter in Path.run to Path.fit_to_reference added - Fixed typo in PSAnalysis.generate_paths referring incorrectly to directory for fitted trajectories as trj_fit rather than fitted_trajs - Added default_path_basename parameter to PSAnalysis and inits to "path" - Added default prefix in PSAnalysis for fitted paths and trajectories; fixed save_paths to use default prefix and path basename if prefix and filename are not provided - test_load now passes pytest - Updated changelog with fix to PSA
Hi all,
One issue with code coverage in PSA is the amount of code dedicated to plotting the results of the path similarity analysis. I'd like to get people's opinions on whether this code should exist to as is, or if we should consider moving it to an example notebook outside of the class. In my limited experience with the analysis modules, it seems like the place for demonstrating how to aggregate and plot the data should be documentation, and not the actual code. There are around 300/1925 lines dedicated to plotting in the PSA module.
The text was updated successfully, but these errors were encountered: