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

[WIP] Refactor software: step 1 #26

Merged
merged 52 commits into from
Jul 9, 2020
Merged

Conversation

gmaze
Copy link
Member

@gmaze gmaze commented Jul 7, 2020

About Refactoring

Refactoring the software can be done in 2 steps:

  1. Re-organise all functions into sub-modules, no changes in function name or content
  2. Optimise sub-modules and functions to make them pythonic, and more efficient

This PR is a proposition for step 1: a new structure of the library where functions are now organized into sub-modules.

New positioning of functions

Note that functions name are not changed !

  • pyowc/core

    • stats.py: brk_pt_fit, build_cov, covarxy_pv, covar_xyt_pv, noise_variance, signal_variance, fit_cond, nlbpfun
    • finders.py: find_10thetas, find_25boxes, find_besthit, find_ellipse, nearest_neighbour
  • pyowc/data

    • fetchers.py: get_region_data, get_region_hist_locations, get_data, get_topo_grid
    • wrangling.py: interp_climatology, map_data_grid
  • pyowc/plot

    • dashboard.py: plot_diagnostics
    • plots.py: cal_sal_curve_plot, sal_var_plot, t_s_profile_plot, theta_sal_plot, trajectory_plot
    • utils.py: create_dataframe
  • pyowc/calibration.py: update_salinity_mapping, calc_piecewisefit

  • pyowc/configuration.py: load_configuration, set_calseries, print_cfg

  • pyowc/tests # Contain all the unit tests !

  • pyowc/utilities.py: change_dates, cal2dec, potential_vorticity, wrap_longitudes, sorter, spatial_correlation

Usage

With this structure, we have a more elegant:

import pyowc as owc
FLOAT_NAME = "3901960"
USER_CONFIG = owc.configuration.load() # fetch the configuration and parameters set by the user
owc.calibration.update_salinity_mapping("/", FLOAT_NAME, USER_CONFIG)
owc.configuration.set_calseries("/", FLOAT_NAME, USER_CONFIG)
owc.calibration.calc_piecewisefit("/", FLOAT_NAME, USER_CONFIG)
owc.plot.dashboard("/", FLOAT_NAME, USER_CONFIG)

Remains to be done

  • Settle on the sub-module names and content
  • Consolidate unit testing

@gmaze gmaze marked this pull request as draft July 7, 2020 07:53
pyowc/calibration.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 8, 2020

Codecov Report

Merging #26 into master will decrease coverage by 3.39%.
The diff coverage is 86.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #26      +/-   ##
==========================================
- Coverage   91.38%   87.99%   -3.40%     
==========================================
  Files          65       12      -53     
  Lines        3145     1957    -1188     
==========================================
- Hits         2874     1722    -1152     
+ Misses        271      235      -36     
Flag Coverage Δ
#unittests 87.99% <86.36%> (-3.40%) ⬇️
Impacted Files Coverage Δ
setup.py 0.00% <0.00%> (ø)
start_with_pycharm.py 0.00% <0.00%> (ø)
pyowc/calibration.py 85.93% <76.38%> (ø)
pyowc/core/stats.py 80.77% <80.77%> (ø)
pyowc/configuration.py 84.72% <84.72%> (ø)
pyowc/data/fetchers.py 90.69% <90.69%> (ø)
pyowc/plot/plots.py 92.80% <92.80%> (ø)
pyowc/core/finders.py 93.84% <93.84%> (ø)
pyowc/data/wrangling.py 100.00% <100.00%> (ø)
pyowc/plot/dashboard.py 100.00% <100.00%> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fef54bb...1f443a4. Read the comment docs.

gmaze added 6 commits July 8, 2020 18:30
And also fixing these l#!!%$#%# path creation with "+" instead of a proper os.path.sep
also moved sel_calseries to configuration.py
@gmaze
Copy link
Member Author

gmaze commented Jul 8, 2020

I think "calc_piecewisefit" should also go into the calibration.py module

@gmaze gmaze marked this pull request as ready for review July 9, 2020 08:38
new_structure_outline.md Outdated Show resolved Hide resolved
new_structure_outline.md Outdated Show resolved Hide resolved
pyowc/configuration.py Show resolved Hide resolved
README.md Show resolved Hide resolved
docs/Tryit.ipynb Show resolved Hide resolved
pyowc/core/stats.py Outdated Show resolved Hide resolved
@edsmall-bodc
Copy link
Collaborator

Apart from the above suggested changes, this looks good to me

@gmaze gmaze mentioned this pull request Jul 9, 2020
14 tasks
@edsmall-bodc edsmall-bodc merged commit 4391ae3 into master Jul 9, 2020
@kamwal kamwal deleted the refactor-configuration branch April 14, 2021 10:41
@kamwal kamwal restored the refactor-configuration branch April 14, 2021 10:42
@kamwal kamwal deleted the refactor-configuration branch April 14, 2021 10:42
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.

2 participants