willfurnass commented 5 years ago

I've refactored __init__.py into several modules. Now __init__.py only effectively exports the functions and classes we want to present as the public API; it contains very little code. The list of modules in the package is now as follows:

$ ls -1 mumot/*py
mumot/consts.py
mumot/controllers.py
mumot/defaults.py
mumot/exceptions.py
mumot/__init__.py
mumot/models.py
mumot/utils.py
mumot/_version.py
mumot/views.py

This PR also includes many changes relating to linting/basic refactoring plus some additional type hints.

Closes #173

(Sorry, this is a PR From Hell)

codecov-io commented 5 years ago

Codecov Report

Merging #337 into master will decrease coverage by 0.19%. The diff coverage is 74.54%.

@@            Coverage Diff            @@
##           master     #337     +/-   ##
=========================================
- Coverage   68.92%   68.73%   -0.2%     
=========================================
  Files           2        9      +7     
  Lines        5696     5754     +58     
  Branches     1556     1572     +16     
=========================================
+ Hits         3926     3955     +29     
- Misses       1382     1410     +28     
- Partials      388      389      +1

Impacted Files	Coverage Δ
mumot/__init__.py	`84.61% <ø> (+15.7%)`	:arrow_up:
mumot/_version.py	`100% <ø> (ø)`	:arrow_up:
mumot/models.py	`71.82% <ø> (ø)`
mumot/views.py	`65.96% <ø> (ø)`
mumot/consts.py	`100% <100%> (ø)`
mumot/defaults.py	`64.28% <64.28%> (ø)`
mumot/utils.py	`67.7% <67.7%> (ø)`
mumot/controllers.py	`77.82% <77.82%> (ø)`
mumot/exceptions.py	`90% <90%> (ø)`
... and 7 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 6df67f5...cd30470. Read the comment docs.

jarmarshall commented 5 years ago

@joefresna ’s comments seems sensible to me...

On 15 Aug 2019, at 11:06, Andreagiovanni Reina notifications@github.com wrote:

@joefresna commented on this pull request.

Hi, some comments on the refactoring in separate modules:

should def _raiseModelError (now in models) go into the expections.py module?

should class NetworkType (now in models) go into the consts.py?

I think the functions def _decodeNetworkTypeFromString() and _encodeNetworkTypeToString (now in models) should go into the utils.py.

I think the functions _deriveODEsFromRules, _deriveMasterEquation, and _doVanKampenExpansion (now in models) should go into the views.py

I think LINE_COLOR_LIST and MULTIPLOT_COLUMNS (now in views) should go into the consts.py

I think that def _round_to_1 and def _make_autopct (now in views) should go into the utils.py.

I am not sure where is the proper place for the "get" functions which are now in the models.py (I think it's ok), and the plotting functions which are now in the views.py and might go in the utils.py.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/DiODeProject/MuMoT/pull/337?email_source=notifications&email_token=AD6OR6RJAFKPQFRA5XTIW5DQEUTB3A5CNFSM4ILNDXI2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCBUYYJQ#pullrequestreview-275352614, or mute the thread https://github.com/notifications/unsubscribe-auth/AD6OR6WGUHTDEY4FP2POA5TQEUTB3ANCNFSM4ILNDXIQ.

willfurnass commented 5 years ago

[X] Move _round_to_1 and _make_autopct funcs from view.py to utils.py
[X] Move LINE_COLOR_LIST and MULTIPLOT_COLUMNS constants from views module to consts module
[X] Move NetworkType class from models.py to consts.py
[X] Move _decodeNetworkTypeFromString and _encodeNetworkTypeToString from models.py to utils.py
[X] Move _raiseModelError func from models.py to exceptions.py
[x] Move _deriveODEsFromRules, _deriveMasterEquation, and _doVanKampenExpansion funcs from models.py to views.py

willfurnass commented 5 years ago

@joefresna Happy with the interactive behavior (widgets and plots) for the HEAD of this branch?

willfurnass commented 5 years ago

And thanks for the review!

joefresna commented 5 years ago

@joefresna Happy with the interactive behavior (widgets and plots) for the HEAD of this branch?

How can I checkout this branch?

git checkout split_module
error: pathspec 'split_module' did not match any file(s) known to git

willfurnass commented 5 years ago

git remote add willfurnass git@github.com:willfurnass/MuMoT.git
git remote -v
git fetch --prune --all
git branch -vva
git checkout split_module

joefresna commented 5 years ago

JF-MacBook:MuMoT joefresna$ git remote -v
origin  https://github.com/DiODeProject/MuMoT.git (fetch)
origin  https://github.com/DiODeProject/MuMoT.git (push)
willfurnass git@github.com:willfurnass/MuMoT.git (fetch)
willfurnass git@github.com:willfurnass/MuMoT.git (push)
JF-MacBook:MuMoT joefresna$ git fetch --prune --all
Fetching origin
Fetching willfurnass
git@github.com: Permission denied (publickey).
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.
error: Could not fetch willfurnass

willfurnass commented 5 years ago

@joefresna If you haven't set up SSH access to GitHub then you'll need to now run the following to change the URL for the willfurnass remote so it uses HTTPS instead of SSH:

git remote set-url willfurnass https://github.com/willfurnass/MuMoT.git
git remote -v
git fetch --prune --all
git branch -vva
git checkout split_module

joefresna commented 5 years ago

Thanks, @willfurnass , I now solved and I can run this branch.

@joefresna Happy with the interactive behavior (widgets and plots) for the HEAD of this branch?

I checked the widgets and plots and the results look correct, and interactivity is working fine.

I guess the refactoring changes are now completed, and from my side I'm happy to proceed with the Merging. Thanks.

DiODeProject / MuMoT

Refactor into separate modules (and lint) #337

Codecov Report