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

Just copy the contrib files #6

Merged
merged 2 commits into from
Sep 21, 2023
Merged

Just copy the contrib files #6

merged 2 commits into from
Sep 21, 2023

Conversation

rhaps0dy
Copy link
Collaborator

@rhaps0dy rhaps0dy commented Sep 21, 2023

This PR copies some files from stable-baselines3-contrib. They are the files that #5 modifies.

@rhaps0dy rhaps0dy marked this pull request as ready for review September 21, 2023 22:13
Copy link

@dan-pandori dan-pandori left a comment

Choose a reason for hiding this comment

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

LGTM, but can you add some more details on the long term plans for these files and the files they are cloned from?

In particular, will these files supersede the old files (meaning that the old files will eventually be deleted)? Or will the two live in parallel indefinitely?

Copy link

@dan-pandori dan-pandori left a comment

Choose a reason for hiding this comment

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

Actually, I just realized that CI is failing for these changes. Was your plan to fix the CI failures as part of this, or in a followup?

@rhaps0dy
Copy link
Collaborator Author

The CI failures should be fixed in #5. The only purpose of this PR is to have a point of reference for #5.

These files will be heavily modified and thus the ability to merge upstream is not helpful. At the same time, upstream has a lot of tests that are just copied from stable-baselines3, so spiritually it's meant to live in this repo.

l these files supersede the old files (meaning that the old files will eventually be deleted)? Or will the two live in parallel indefinitely?

The 'old files' are in https://github.com/rhaps0dy/stable-baselines3-contrib, which will be completely removed from the learned-planners parent repo.

@rhaps0dy rhaps0dy merged commit 8ccaa72 into master Sep 21, 2023
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.

2 participants