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

Add TemporaryEnvironment context manager #16

Merged
merged 7 commits into from
Jun 18, 2024
Merged

Add TemporaryEnvironment context manager #16

merged 7 commits into from
Jun 18, 2024

Conversation

pmrv
Copy link
Contributor

@pmrv pmrv commented Jun 18, 2024

This class sets and restores environment variables within the context. I'd like to use this in pyiron_base, but thought it'd be nice here as well.

Tests generated with AI, it's not my fault if they break!!1!

This class sets and restores environment variables within the context.
I'd like to use this in pyiron_base, but thought it'd be nice here as
well.
Copy link

Binder 👈 Launch a binder notebook on branch pyiron/pyiron_snippets/tempenv

tests/unit/test_tempenv.py Outdated Show resolved Hide resolved
Copy link

codacy-production bot commented Jun 18, 2024

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.32% (target: -1.00%) 100.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (ce59a49) 434 397 91.47%
Head commit (a8f3e65) 451 (+17) 414 (+17) 91.80% (+0.32%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#16) 17 17 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences


🚀 Don’t miss a bit, follow what’s new on Codacy.

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

@pmrv pmrv added the format_black Trigger the black formatting bot label Jun 18, 2024
@coveralls
Copy link

coveralls commented Jun 18, 2024

Pull Request Test Coverage Report for Build 9567860514

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 91.796%

Totals Coverage Status
Change from base Build 9420542945: 0.3%
Covered Lines: 414
Relevant Lines: 451

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 18, 2024

Pull Request Test Coverage Report for Build 9567948767

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 91.796%

Totals Coverage Status
Change from base Build 9420542945: 0.3%
Covered Lines: 414
Relevant Lines: 451

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 18, 2024

Pull Request Test Coverage Report for Build 9567969556

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 91.796%

Totals Coverage Status
Change from base Build 9420542945: 0.3%
Covered Lines: 414
Relevant Lines: 451

💛 - Coveralls

@pmrv pmrv requested a review from liamhuber June 18, 2024 15:51
@jan-janssen
Copy link
Member

For the application on executables, I am wondering if it is easier to set the environment variables only to the subprocess: https://docs.python.org/3/library/subprocess.html#popen-constructor - Popen(env)

Copy link
Member

@liamhuber liamhuber left a comment

Choose a reason for hiding this comment

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

I like the plan, raw code looks fine to me, just need to get the surrounding infrastructure working.

Tests generated with AI, it's not my fault if they break!!1!

From the "!!1!" I assume this is a little tongue-in-cheek. I do find it super useful to generate initial tests from GPT, but setting aside your frivolity we do need to actually make sure they are both meaningful and complete. In this case, the AI tests actually failed -- I haven't dug in deeply yet to why, but that's a great indicator they actually found something meaningful! In terms of completeness, I read through and don't see any extra cases that are needed.

The docstring will fail a doctest because it doesn't actually show the output. I'm not sure a priori what formatting will be necessary here, but it should be simple enough for me to add when I start the doctests running -- so if I get that done before you merge here, let's fix it, but otherwise no need to wait on it and the example is already human-ready.

tests/unit/test_tempenv.py Show resolved Hide resolved
pyiron_snippets/tempenv.py Show resolved Hide resolved
Co-authored-by: Liam Huber <[email protected]>
@coveralls
Copy link

coveralls commented Jun 18, 2024

Pull Request Test Coverage Report for Build 9570213602

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 91.796%

Totals Coverage Status
Change from base Build 9420542945: 0.3%
Covered Lines: 414
Relevant Lines: 451

💛 - Coveralls

@liamhuber liamhuber added format_black Trigger the black formatting bot and removed format_black Trigger the black formatting bot labels Jun 18, 2024
@pmrv
Copy link
Contributor Author

pmrv commented Jun 18, 2024

Tests generated with AI, it's not my fault if they break!!1!

From the "!!1!" I assume this is a little tongue-in-cheek. I do find it super useful to generate initial tests from GPT, but setting aside your frivolity we do need to actually make sure they are both meaningful and complete. In this case, the AI tests actually failed -- I haven't dug in deeply yet to why, but that's a great indicator they actually found something meaningful! In terms of completeness, I read through and don't see any extra cases that are needed.

Yeah, I prodded GPT a bit and modified the results, but it was enough to get me over the initial activation barrier.

Copy link

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation Diff coverage
+0.32% (target: -1.00%) 100.00%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (ce59a49) 434 397 91.47%
Head commit (527bd4b) 451 (+17) 414 (+17) 91.80% (+0.32%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#16) 17 17 100.00%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences


🚀 Don’t miss a bit, follow what’s new on Codacy.

Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more

@coveralls
Copy link

coveralls commented Jun 18, 2024

Pull Request Test Coverage Report for Build 9570305303

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 91.796%

Totals Coverage Status
Change from base Build 9420542945: 0.3%
Covered Lines: 414
Relevant Lines: 451

💛 - Coveralls

@liamhuber liamhuber self-requested a review June 18, 2024 18:53
@pmrv pmrv merged commit bce8017 into main Jun 18, 2024
18 of 19 checks passed
@pmrv pmrv deleted the tempenv branch June 18, 2024 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
format_black Trigger the black formatting bot
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants