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

fix: Return functionality of yadage.utils.coerce_data_arg to state_provider_from_string #117

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

matthewfeickert
Copy link
Member

@matthewfeickert matthewfeickert commented Feb 7, 2022

Resolves #116

PR #92 moved and generalized functionality from yadage/state_providers/__init__.py to yadage/utils.py with the introduction of yadage.utils.coerce_data_arg. However, it never added that functionality back into yadage/state_providers/__init__.py

$ git show b03a637f2afa53f5967472e5df7d17b229905e0a -- yadage/state_providers/
commit b03a637f2afa53f5967472e5df7d17b229905e0a
Author: Lukas <lukas.heinrich@gmail.com>
Date:   Fri Nov 15 03:20:25 2019 +0100

    coerce dataarg (#92)
    
    * coerce data arg

diff --git a/yadage/state_providers/__init__.py b/yadage/state_providers/__init__.py
index ea0840a..a2c65bd 100644
--- a/yadage/state_providers/__init__.py
+++ b/yadage/state_providers/__init__.py
@@ -80,8 +80,6 @@ def fromenv_provider(dataarg, dataopts):
 
 def state_provider_from_string(dataarg, dataopts=None):
     dataopts = dataopts or {}
-    if len(dataarg.split(":", 1)) == 1:
-        dataarg = "local:" + dataarg
     for k in providersetup_handlers.keys():
         if dataarg.startswith(k):
             return providersetup_handlers[k](dataarg, dataopts)
(venv) feickert@ThinkPad-X1:~/Code/GitHub/yadage/yadage$ git show b03a637f2afa53f5967472e5df7d17b229905e0a -- yadage/utils.py
commit b03a637f2afa53f5967472e5df7d17b229905e0a
Author: Lukas <lukas.heinrich@gmail.com>
Date:   Fri Nov 15 03:20:25 2019 +0100

    coerce dataarg (#92)
    
    * coerce data arg

diff --git a/yadage/utils.py b/yadage/utils.py
index 1859174..9c00c48 100644
--- a/yadage/utils.py
+++ b/yadage/utils.py
@@ -247,3 +247,9 @@ def pointerize(data, asref=False, stepid=None):
         )
 
     return data.asrefs(callback=callback)
+
+
+def coerce_data_arg(dataarg):
+    if len(dataarg.split(":", 1)) == 1:
+        dataarg = "local:" + dataarg
+    return dataarg

This PR adds that functionality back and simplifies some additional logic.

Squash and merge commit message

* Return functionality of 'yadage.utils.coerce_data_arg' to
'yadage.state_providers.state_provider_from_string'
   - This functionality was factored out in PR #92, but wasn't replaced
with a call to yadage.utils.coerce_data_arg.
   - Amends PR #92
* Simplify logic around metadir assignment as general approach should be
independent of value of dataarg.
* Ensure local provider setup uses 'local:' for string name.
* Improve state_provider_from_string's RuntimeError message.

@matthewfeickert matthewfeickert self-assigned this Feb 7, 2022
@matthewfeickert matthewfeickert marked this pull request as draft February 8, 2022 00:04
Comment on lines 49 to 56
metadir = (
kwargs["metadir"]
if "metadir" in kwargs.keys() and kwargs["metadir"] is not None
else f"{kwargs['dataarg']}/_yadage/"
)
if dataopts.get("overwrite") and os.path.exists(metadir):
shutil.rmtree(metadir)

Copy link
Member Author

Choose a reason for hiding this comment

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

This part is currently problematic. With this in it is currently failing

pytest -sx tests/test_utilcli.py

but if I remove it then using this yadage code with

recast run testing/busyboxtest --backend local --tag hello

fails.

@matthewfeickert matthewfeickert force-pushed the fix/get-working branch 2 times, most recently from 9b96bbb to 67f04d7 Compare February 9, 2022 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

coerce dataarg PR causing regression in utils.prepare_meta
1 participant