Skip to content
This repository was archived by the owner on Aug 28, 2025. It is now read-only.

Conversation

@phlippe
Copy link
Contributor

@phlippe phlippe commented Nov 26, 2022

What does this PR do?

In the UvA DL Tutorial 8 (Deep Autoencoder), the generation callback used a deprecated and now removed API of on_epoch_end. This pull request updates it to on_train_epoch_end to be compatible with the new PL 1.8 version.

Did you have fun?

Make sure you had fun coding 🙃

@codecov
Copy link

codecov bot commented Nov 26, 2022

Codecov Report

Merging #210 (c6337ce) into main (5419bb2) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@         Coverage Diff         @@
##           main   #210   +/-   ##
===================================
  Coverage    73%    73%           
===================================
  Files         2      2           
  Lines       382    382           
===================================
  Hits        280    280           
  Misses      102    102           

@Borda
Copy link
Contributor

Borda commented Jan 3, 2023

@phlippe seems we have finally fixed the ci/cd and so we are fine to proceed... 🐰

@Borda Borda self-assigned this Jan 3, 2023
@Borda
Copy link
Contributor

Borda commented Jan 3, 2023

btw, shall we in such a case also bump the min version? 🦦

@Borda Borda merged commit 52675fd into Lightning-AI:main Jan 4, 2023
@Borda Borda added the bug / fix Something isn't working label Jan 4, 2023
@phlippe
Copy link
Contributor Author

phlippe commented Jan 4, 2023

Thanks @Borda for fixing the CI and merging the pull request!
Pushing up the minimum version sounds good, I noticed that the notebooks are still using progress_bar_refresh_rate which also became deprecated lately in Lightning. I will send another pull request with this update, then all notebooks should run smoothly with the newest Lightning version

@Borda
Copy link
Contributor

Borda commented Jan 4, 2023

@phlippe shall we bump all classes to use PL 1.8 as minimal version? What do you think?

@phlippe
Copy link
Contributor Author

phlippe commented Jan 4, 2023

Yes, agreed, there have been a few API changes since 1.4 which would be good to enforce with 1.8 (e.g. accelerator selection and progress bar). I think we may have to merge #222 beforehand though

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.

2 participants