Skip to content

Conversation

@Vaspra
Copy link
Contributor

@Vaspra Vaspra commented Sep 26, 2019

I was having an issue with meta_curriculum attempting to generate curricula instances from Unity-made curricula/MyThingLearning/MyThingLearning.json.meta files. Obviously the meta files are not intended to be used as .json sources.

The change simply excludes filenames ending in .meta.

@CLAassistant
Copy link

CLAassistant commented Sep 26, 2019

CLA assistant check
All committers have signed the CLA.

@chriselion chriselion changed the base branch from master to develop September 26, 2019 05:08
@chriselion chriselion self-requested a review September 26, 2019 05:11
@chriselion
Copy link
Contributor

Hi @Vaspra
Hope you don't mind, I changed the target branch from master to develop. I'll take a closer look tomorrow..

@Vaspra
Copy link
Contributor Author

Vaspra commented Sep 26, 2019 via email

for curriculum_filename in os.listdir(curriculum_folder):
for curriculum_filename in\
[fn for fn in os.listdir(curriculum_folder)\
if not fn.endswith('.meta')]:
Copy link
Contributor

Choose a reason for hiding this comment

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

How about just moving this check inside the loop, like

if curriculum_filename.endswith('.meta'):
    continue

?

Copy link
Contributor

@chriselion chriselion left a comment

Choose a reason for hiding this comment

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

Hi Doug,
Thanks for the submission. I left a comment; I think the check would be better in the loop.

We just turned on continuous integration (CircleCI) for forked pull requests like this one; I think next time you push it will run. The formatter (black) will definitely try to reformat that for loop anyway.

@Vaspra Vaspra closed this Oct 1, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants