Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Override add_argparse_args in the FlashTrainer #343

Merged
merged 2 commits into from
May 26, 2021

Conversation

carmocca
Copy link
Contributor

@carmocca carmocca commented May 26, 2021

What does this PR do?

Hacky fix, but I want to avoid having to implement support for subclasses in Lightning. There is jsonargparse for that.

Fixes #342

Before submitting

  • Was this discussed/approved via a Github issue? (no need for typos and docs improvements)
  • Did you read the contributor guideline, Pull Request section?
  • Did you make sure your PR does only one thing, instead of bundling different changes together?
  • [n/a] Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests? [not needed for typos/docs]
  • Did you verify new and existing tests pass locally with your changes?
  • If you made a notable change (that affects users), did you update the CHANGELOG?

PR review

  • Is this pull request ready for review? (if not, please submit in draft mode)

@carmocca carmocca added the bug / fix Something isn't working label May 26, 2021
@carmocca carmocca self-assigned this May 26, 2021
@codecov
Copy link

codecov bot commented May 26, 2021

Codecov Report

Merging #343 (0366f44) into master (2113c11) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #343      +/-   ##
==========================================
+ Coverage   88.59%   88.61%   +0.01%     
==========================================
  Files          80       80              
  Lines        4061     4066       +5     
==========================================
+ Hits         3598     3603       +5     
  Misses        463      463              
Flag Coverage Δ
unittests 88.61% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
flash/core/trainer.py 86.76% <100.00%> (+1.05%) ⬆️

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 2113c11...0366f44. Read the comment docs.

Copy link
Collaborator

@ethanwharris ethanwharris left a comment

Choose a reason for hiding this comment

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

Haha, it is hacky but it works, LGTM 😃

@kaushikb11 kaushikb11 enabled auto-merge (squash) May 26, 2021 16:13
@kaushikb11 kaushikb11 merged commit 6fcfe2e into master May 26, 2021
@kaushikb11 kaushikb11 deleted the bugfix/support-add-argparse-args branch May 26, 2021 16:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug / fix Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

flash.Trainer.add_argparse_args(parser) doesn't register args to arg parse
3 participants