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

rename peak_signal_noise_ratio #732

Merged
merged 28 commits into from
Jan 12, 2022
Merged

rename peak_signal_noise_ratio #732

merged 28 commits into from
Jan 12, 2022

Conversation

AIMistakes
Copy link
Contributor

@AIMistakes AIMistakes commented Jan 8, 2022

What does this PR do?

Change
'PSNR' to 'PeakSignalNoiseRatio'
psnr to peak_signal_noise_ratio

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 to update the docs?
  • Did you write any new necessary tests?

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Did you have fun?

Yep 😄

Make sure you had fun coding 🙃

@AIMistakes AIMistakes changed the title psnr --> peak_signal_noise_ratio psnr changed to peak_signal_noise_ratio Jan 8, 2022
@AIMistakes AIMistakes changed the title psnr changed to peak_signal_noise_ratio Unify peak_signal_noise_ratio Jan 8, 2022
@AIMistakes
Copy link
Contributor Author

AIMistakes commented Jan 8, 2022

Linked to issue: Unify functional metrics names #729 . @Borda

@codecov
Copy link

codecov bot commented Jan 8, 2022

Codecov Report

Merging #732 (ff67ba8) into master (4c87069) will increase coverage by 0%.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #732   +/-   ##
=====================================
  Coverage      95%    95%           
=====================================
  Files         170    170           
  Lines        6742   6751    +9     
=====================================
+ Hits         6402   6411    +9     
  Misses        340    340           

@SkafteNicki
Copy link
Member

Hi @Fariborzzz, it seems this PR is changing a lot more file than intended. Could you fix that?

@Borda Borda added API / design refactoring refactoring and code health labels Jan 8, 2022
@Borda
Copy link
Member

Borda commented Jan 8, 2022

Hi @Fariborzzz, it seems this PR is changing a lot more file than intended. Could you fix that?

the case seems to be that you have by default some different file permissions which yielded in all project changes, for next time is better to set your git to ignore file permissions before committing changes

git config core.fileMode false

@Borda
Copy link
Member

Borda commented Jan 8, 2022

@Fariborzzz shall be fixed now 🐰

@AIMistakes
Copy link
Contributor Author

the case seems to be that you have by default some different file permissions which yielded in all project changes, for next time is better to set your git to ignore file permissions before committing changes

Thank you @SkafteNicki and @Borda. Sure, I'll set up git ignore for my next commits 😃!
This was the first commit, hope the next ones are smoother.

@Borda Borda mentioned this pull request Jan 8, 2022
12 tasks
@SkafteNicki
Copy link
Member

SkafteNicki commented Jan 8, 2022

Hi @Fariborzzz,
Could you also rename the modular class: PSNR -> PeakSignalNoiseRatio ?

Copy link
Member

@Borda Borda left a comment

Choose a reason for hiding this comment

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

deprecation warnings & back compatibility

@Borda Borda added this to the v0.7 milestone Jan 8, 2022
@mergify mergify bot added the has conflicts label Jan 8, 2022
@AIMistakes AIMistakes marked this pull request as ready for review January 10, 2022 18:09
@Borda
Copy link
Member

Borda commented Jan 10, 2022

@Fariborzzz could you pls resolve the conflict or shall I do it for you? 🐰

@Borda Borda assigned Borda and unassigned SkafteNicki Jan 10, 2022
@Borda Borda added Important milestonish and removed Important milestonish labels Jan 10, 2022
@AIMistakes AIMistakes requested a review from Borda January 10, 2022 21:12
@mergify mergify bot removed the has conflicts label Jan 11, 2022
@Borda Borda enabled auto-merge (squash) January 12, 2022 11:08
@mergify mergify bot added ready and removed has conflicts labels Jan 12, 2022
@Borda Borda disabled auto-merge January 12, 2022 18:40
@Borda Borda merged commit b4ceaea into Lightning-AI:master Jan 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API / design ready refactoring refactoring and code health
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants