attempt for #451

[SoumyajitKarmakar]

Modified the env.reset() function to add the reward_space and observation_space parameters. Modified the reset() func of the subclasses. Added a OptionalArgumentValue class in gym_type_hints.py for the default value.

Fixes #451

[ChrisCummins]

Hi @SoumyajitKarmakar, sounds great! Would you like me to review your changes or are you still working on this?

Cheers,
Chris

[codecov-commenter]

Codecov Report

Merging #650 (f842508) into development (3c21aee) will decrease coverage by 14.60%.
The diff coverage is 88.88%.

@@               Coverage Diff                @@
##           development     #650       +/-   ##
================================================
- Coverage        88.13%   73.52%   -14.61%     
================================================
  Files              116      119        +3     
  Lines             7372     7483      +111     
================================================
- Hits              6497     5502      -995     
- Misses             875     1981     +1106     

Impacted Files	Coverage Δ
...ompiler_gym/service/client_service_compiler_env.py	88.05% <75.00%> (-2.51%)	⬇️
compiler_gym/envs/gcc/gcc_env.py	100.00% <100.00%> (ø)
compiler_gym/util/gym_type_hints.py	100.00% <100.00%> (ø)
compiler_gym/leaderboard/__init__.py	0.00% <0.00%> (-100.00%)	⬇️
compiler_gym/leaderboard/llvm_instcount.py	0.00% <0.00%> (-92.79%)	⬇️
compiler_gym/util/minimize_trajectory.py	0.00% <0.00%> (-92.63%)	⬇️
compiler_gym/bin/validate.py	0.00% <0.00%> (-87.10%)	⬇️
compiler_gym/bin/manual_env.py	0.00% <0.00%> (-80.74%)	⬇️
compiler_gym/bin/service.py	0.00% <0.00%> (-75.81%)	⬇️
compiler_gym/wrappers/llvm.py	33.33% <0.00%> (-64.59%)	⬇️
... and 51 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c21aee...f842508. Read the comment docs.

[SoumyajitKarmakar]

Hi @ChrisCummins,
Yes, please review my changes and let me know if it requires any additional modifications.

[ChrisCummins]

Hi @SoumyajitKarmakar, thanks for your patch, your changes look good!

Since you are adding new features, could you please add unit tests to cover these new features? I think tests/compiler_env_test.py might be a good place to add them, and you can take a look at the existing tests for inspiration.

A couple of other minor nitpicks left inline. Once you push these changes, please ping me for another review.

Cheers,
Chris

[SoumyajitKarmakar]

Hi @ChrisCummins,
Please review the latest changes. I have removed the unnecessary commented lines and added some tests.
It is my first time writing tests for a software. I have added 4 simple tests which should hopefully cover the new features. Please let me know if any other kinds of tests are required.

[ChrisCummins]

Thanks for adding the tests! The test logic looks good, you just need to make sure to convert the env.reward_space and env.observation_space attributes to strings for comparison.

Once you fix that, this looks ready to merge.

Cheers,
Chris

[ChrisCummins]

Thanks @SoumyajitKarmakar for fixing this :) This LGTM now, but holding off on merging until I fix #657.

Cheers,
Chris

[ChrisCummins]

Ah, looks like I gave you bad advice on how to fix the failing tests. Not your fault, I'll patch this up myself and merge via #659.

Thanks for contributing to CompilerGym @SoumyajitKarmakar! 🎉

Cheers,
Chris

[SoumyajitKarmakar]

It was a pleasure working with you.