Skip to content

Use foreign keys for model/proto version references in Experiment table#250

Merged
sroderick-g5sro merged 19 commits intomasterfrom
241-refactor-running-exp-link
Feb 11, 2020
Merged

Use foreign keys for model/proto version references in Experiment table#250
sroderick-g5sro merged 19 commits intomasterfrom
241-refactor-running-exp-link

Conversation

@sroderick-g5sro
Copy link
Copy Markdown
Contributor

@sroderick-g5sro sroderick-g5sro commented Feb 6, 2020

Preliminary refactor suggested in #241.

@sroderick-g5sro sroderick-g5sro self-assigned this Feb 6, 2020
Comment thread weblab/entities/views.py Outdated
@codecov-io
Copy link
Copy Markdown

codecov-io commented Feb 10, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@9a225a9). Click here to learn what that means.
The diff coverage is 92.2%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master    #250   +/-   ##
========================================
  Coverage          ?   94.6%           
========================================
  Files             ?      65           
  Lines             ?    2656           
  Branches          ?     278           
========================================
  Hits              ?    2513           
  Misses            ?     104           
  Partials          ?      39
Impacted Files Coverage Δ
weblab/conftest.py 99.3% <ø> (ø)
weblab/experiments/processing.py 92.5% <ø> (ø)
weblab/entities/repository.py 97.4% <ø> (ø)
weblab/entities/processing.py 93.5% <ø> (ø)
weblab/experiments/views.py 98.3% <100%> (ø)
weblab/experiments/models.py 99% <100%> (ø)
weblab/entities/models.py 94.6% <100%> (ø)
weblab/core/recipes.py 100% <100%> (ø)
weblab/repocache/models.py 100% <100%> (ø)
weblab/entities/templatetags/entities.py 100% <100%> (ø)
... and 2 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 9a225a9...ddee99c. Read the comment docs.

@jonc125 jonc125 changed the title 241 refactor running exp link Use foreign keys for model/proto version references in Experiment table Feb 11, 2020
Copy link
Copy Markdown
Contributor

@jonc125 jonc125 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good! I think all my suggestions are minor ones - either slight tidies to tests, or slight extra refactoring that's now possible.

Comment thread weblab/conftest.py Outdated
Comment thread weblab/entities/templatetags/entities.py Outdated
Comment thread weblab/entities/tests/test_views.py Outdated
Comment thread weblab/entities/tests/test_views.py Outdated
Comment thread weblab/entities/tests/test_views.py
Comment thread weblab/experiments/tests/test_views.py Outdated
Comment thread weblab/experiments/tests/test_views.py Outdated
Comment thread weblab/experiments/tests/test_views.py Outdated
Comment thread weblab/experiments/tests/test_views.py
Comment thread weblab/experiments/tests/test_views.py
@jonc125 jonc125 mentioned this pull request Feb 11, 2020
13 tasks
@sroderick-g5sro
Copy link
Copy Markdown
Contributor Author

thank you for your detailed comments - I hope I have got everything.

Copy link
Copy Markdown
Contributor

@jonc125 jonc125 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Just some flake8 issues to sort. You might want to set up the git pre-commit hook to catch those for you?

flake8 --install-hook=git
git config --bool flake8.strict true

@sroderick-g5sro sroderick-g5sro merged commit 55986ca into master Feb 11, 2020
@sroderick-g5sro sroderick-g5sro deleted the 241-refactor-running-exp-link branch February 11, 2020 17:17
MauriceHendrix pushed a commit that referenced this pull request Jul 22, 2021
…-link

Use foreign keys for model/proto version references in Experiment table
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.

3 participants