Rename prototype weight names to comply with PEP8 #5257
Conversation
💊 CI failures summary and remediationsAs of commit 5effa36 (more details on the Dr. CI page): 💚 💚 Looks good so far! There are no failures yet. 💚 💚 This comment was automatically generated by Dr. CI (expand for details).Please report bugs/suggestions to the (internal) Dr. CI Users group. |
There was a problem hiding this comment.
The changes look great @yoshitomo-matsubara. Thanks for your help. I've left only 1 comment that needs to be addressed. Please have a look and let me know.
Finally, could you please add a test on test/test_prototype_models.py to confirm that all available weights have enums that are capitalized? This will help us catch cases were we missed to change them and also enforce this style on the future. You can do this on the test_schema_meta_validation where we already loop throw all the w values of the weights_enum. Thanks in advance.
There was a problem hiding this comment.
@yoshitomo-matsubara All look great. Only one remaining comment on the test and we are good to merge.
You can ignore the 2 failing tests as they are not caused by your PR. One is caused by an unrelated PR that we will revert and one by an upstream bug on Core which is being investigated.
|
@datumbox Thank you for the feedback. I think all the unit tests were passed. |
|
@yoshitomo-matsubara Somehow the test was dropped, so I've just added it back. I'll merge once everything passes. |
There was a problem hiding this comment.
LGTM, thanks for your contribution @yoshitomo-matsubara.
We'll need to update the article to reflect upon the changes. I'll ping the team to do it on my side. :)
Edit: The article was updated pytorch/pytorch.github.io#915
datumbox
changed the title
|
@datumbox |
Summary: * renamed ImageNet weights * renamed COCO weights * renamed COCO with VOC labels weights * renamed Kinetics 400 weights * rename default with DEFAULT * update test * fix typos * update test * update test * update test * indent as w was weight_enum * revert * Adding back the capitalization test Reviewed By: jdsgomes, prabhat00155 Differential Revision: D33739379 fbshipit-source-id: cec33c6bd6be59f4f44bd2adc9906eaa9e1c6696 Co-authored-by: Vasilis Vryniotis <[email protected]>
@datumbox @zhiqwang @pmeier
As discussed in #5088, #5088 (comment) and #5244, I renamed all the weights names, following PEP8.