-
Notifications
You must be signed in to change notification settings - Fork 23
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
Plot Refactoring #263
Open
pabloitu
wants to merge
9
commits into
SCECcode:v1.0.0-candidate
Choose a base branch
from
pabloitu:260-plot-refactoring
base: v1.0.0-candidate
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Plot Refactoring #263
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Harmonized the parameters for all plotting functions. Removed the `plot_args` argument from all functions. Now, default parameters, such as `title`, `figsize` and many other repeating parameters are now found in a global dictionary `DEFAULT_PLOT_ARGS`. All plotting functions now default to these parameters and update a copy of it with **kwargs, thus having control to customize plots. Relevant parameters to the function are left in the function signature. - Added type hints for all signatures. - Removed functions `plot_cumulative_events_versus_time_dev`, `plot_ecdf`, `plot_magnitude_histogram_dev`. - Refactored catalog evaluation plot functions `plot_number_test`, `plot_magnitude_test`, `plot_spatial_test`, `plot_likelihood_test` into a generic `plot_distribution_test` (Should it be renamed plot_consistency_test?). Automatic annotations for each type test are done if required, or when called directly from EvaluationResult subclasses. - The function `plot_poisson_consistency_test` was collapsed onto `plot_consistency_test`, which also included the negative binomial model. - Reworked `plot_magnitude_vs_time`: removed some old functionality and give the possibility to scale the size of each event by its magnitude. Added datetime formatter/locator - Reworked`plot_cumulative_events_versus_time`. Now it has the option to plot with the real datetimes, or hours/days after a given mainshock. - Now plot_magnitude_histogram plots in log scale. - plot_basemap now allows cacheing of downloaded data - Created or refactored from existing functions multiple helper functions to ease unit testing and code readability: `_get_basemap`,`_autosize_scatter`, `_size_map`, `_autoscale_histogram`, `_annotate_distribution_plot`, `_calculate_spatial_extent`, `_create_geo_axes`, `_calculate_marker_size`, `_add_gridlines`, `_define_colormap_and_alpha`, `_add_colorbar`, _process_stat_distribution`. These still needs some review, type hinting and further refactoring if necessary. tests: Completed test coverage of almost all plots.py functions. Added test artifacts for catalog based evaluations (catalogs, forecasts, test results).
This was
linked to
issues
Aug 20, 2024
…he same projection of the cartopy GeoAxes. ft: Added cache handler for plot_basemap. Uses the cache option from cartopy, but removes the cache if a different basemap source is selected (otherwise it would get stuck with the first cached map). tests: added .tif basemap plotting to existing unit tests build: added rasterio as requirement
rasterio is breaking the macos python 3.9 CI build for some reason. Need to fix |
…d unused helper functions
pabloitu
force-pushed
the
260-plot-refactoring
branch
from
August 23, 2024 14:30
8ca37ee
to
c348605
Compare
…ent into the EvaluationResult, as part of the test_distribution attribute: ('negbinom', mean, variance). plot_consistency_test() now does not accept variance as attribute, but it is rather obtained when processing the negative binomial distribution. tests: unittests of consistency test now do not accept variance argument, and negative_binomial mocks contain the second variance parameter as part of the test_distribution
…t is not necessary to set it again when plotting. plot_comparison_test() includes a default legend that explains the symbology therein.
This was
linked to
issues
Aug 27, 2024
Open
…em compatible with sphinx docs. Reordered the index of the API reference docs for plotting functions. Also added EvaluationResult and CartesianGrid2D.get_cartesian, which are referenced by the plot docs. Added cartopy for intersphinx_mapping refac: Renamed plot_spatial_dataset to plot_gridded_dataset (since catalogs are also a spatial datasets)
…of plot_args. Kept the `plot_args` for legacy compatibility. docs: Adapted tutorials to the new plot refactoring. Added catalog_plot to Catalog Operations tutorial. ft: Added higher resolutions for basemap autoscale functions, in case extent is lower.
The refactoring is ready. All the tasks done for this refactoring are explicit in #260. Would be great if @mherrmann3, @bayonato89 and/or @Serra314 can also take a look. The main changes are:
This PR should go to a v1.0.0 candidate branch, since its not backwards compatible. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #260
closes #258
pyCSEP Pull Request Checklist
Please check out the contributing guidelines for some tips
on making pull requests to pyCSEP.
Fixes issue #(please fill in or delete if not needed).
Type of change:
Please delete options that are not relevant.
Checklist: