Adding warning to point people to 529

[JackUrb]

Adding warning on importing mephisto to point to the breaking hydra change Github issue. Couldn't think of another way to deal with getting the word out on user scripts.

[codecov-commenter]

Codecov Report

Merging #530 (8ed4485) into master (1e0a4c2) will decrease coverage by 0.05%.
The diff coverage is 35.71%.

@@            Coverage Diff             @@
##           master     #530      +/-   ##
==========================================
- Coverage   65.60%   65.55%   -0.06%     
==========================================
  Files          79       79              
  Lines        7246     7258      +12     
==========================================
+ Hits         4754     4758       +4     
- Misses       2492     2500       +8     

Impacted Files	Coverage Δ
mephisto/__init__.py	84.61% <ø> (ø)
mephisto/operations/hydra_config.py	75.60% <25.00%> (-20.95%)	⬇️
mephisto/operations/logger_core.py	78.57% <100.00%> (ø)
...tractions/architects/channels/websocket_channel.py	80.23% <0.00%> (-2.33%)	⬇️
mephisto/operations/supervisor.py	78.83% <0.00%> (+0.69%)	⬆️

---

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 1e0a4c2...8ed4485. Read the comment docs.

[pringshia]

• will this error show all the time? wouldn't that create too much noise? is there any way to detect and only show when we can better anticipate that the error may occur?

  • if there's no better solution, i'd suggest changing this from an error red to a warning yellow, as well as with a phase-out plan (e.g. we agree to phase out this warning by version 0.4.x or 0.5.0) so that we can limit the noise. one can imagine the codebase slowly accumulating these warnings over time, i don't think this is scalable
  • I think we should also file an issue with Hydra so that we can figure out if there's a better way to "catch" the error thrown in Running static_test_script.py: Usage of deprecated keyword in package header #488 and display a user-friendly message accordingly (cc/ @omry)

• I think the remediation steps in Breaking change - Hydra 1.1 Upgrade : "Missing @Package directive" #529 could be a little more clearer by highlighting the exact remediation steps

  • perhaps we can add a high level header called "Remediation steps" under "Overview"
  • maybe we can show a sample before/after file & directory structure to show explicitly how the change looks. I find these easier to grok than textual directions. You do link to Updating compatibility for Hydra 1.1 #494 which is helpful, but I think we could also add a sample file change within the issue description

[JackUrb]

I can change to yellow - I was planning to phase this out entirely with 1.0, but can do so earlier if we have any 0.x.0 release before then. Good feedback. Unfortunately I don't have strong ideas for a better way to detect this just yet. Perhaps it would be possible to find the "imported from" location, and see if there's a hydra_configs directory there already? I can look that up.

Will go back to the issue and add more detail though!

[JackUrb]

I ended up keeping the warning red, but changed the location to only surface when registering a script configuration.

[omry]

I think we should also file an issue with Hydra so that we can figure out if there's a better way to "catch" the error thrown in Running static_test_script.py: Usage of deprecated keyword in package header #488 and display a user-friendly message accordingly (cc/ @omry)

I don't know if you can intercept Python warnings issued by other modules.
Generally, Hydra's warning should be friendly enough for most people to figure out how to handle them.

[pringshia]

LGTM!