small fix to #255

[djarecka]

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Summary

small fix of is _existing_file

Checklist

• All tests passing
• I have added tests to cover my changes
• I have updated documentation (if necessary)
• My code follows the code style of this project
  (we are using black: you can pip install pre-commit,
  run pre-commit install in the pydra directory
  and black will be run automatically with each commit)

Acknowledgment

• I acknowledge that this contribution will be available under the Apache 2 license.

[codecov]

Codecov Report

Merging #257 into master will increase coverage by 0.06%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #257      +/-   ##
==========================================
+ Coverage   86.44%   86.51%   +0.06%     
==========================================
  Files          19       19              
  Lines        3047     3047              
  Branches      815      815              
==========================================
+ Hits         2634     2636       +2     
+ Misses        258      256       -2     
  Partials      155      155              

Flag	Coverage Δ
#unittests	86.51% <100.00%> (+0.06%)	⬆️

Impacted Files	Coverage Δ
pydra/engine/helpers_file.py	80.27% <100.00%> (ø)
pydra/engine/task.py	79.68% <0.00%> (+0.78%)	⬆️

---

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 c646892...579fb63. Read the comment docs.

[nicolocin]

FYI this warning showed up when I ran import pydra in Python 3.8.3 but not Python 3.7.6. Not sure if it's something we want to fix?

>>> import pydra
/Users/gablab/Desktop/nlo/pydra/pydra/engine/helpers_file.py:540: SyntaxWarning: "is" with a literal. Did you mean "=="?
  if value is "":

[effigies]

Yeah, I agree that we probably want ==, not is. What was the reasoning for switching?

[nicolocin]

@effigies - there's another warning if we do == which we found in #255

[effigies]

Got it.

[djarecka]

so the problem is with this simple line of code that it gives various errors or warning depending on the type of object:

• if value gives an error when value is a numpy array
• if value == "" if fine with the current numpy, but gives warning that it will not be fine in the future
• and now you're telling me that if value is "" also gives warning, with python this time ;-)

any idea if there is a way to avoid all the warnings without checking the type of value?

[effigies]

I don't think you can avoid it. What are all of the cases that you're trying to catch?

If you want exactly the empty string, then if isinstance(value, str) and value == "" is what you should do. If you want things that serialize to the empty string, then if str(value) == "" will work. This will catch, e.g., np.array("") (but not np.array([""])).

[satra]

the catch22 is that this arises for numpy, but numpy is not a dependency of pydra

[satra]

so we would even have to test if numpy can be imported first. but may not be a bad thing to do.

[djarecka]

alternatively, we can follow @effigies suggestion and check if something is the specific object first. I tried to do it when working on the pr, but I wasn't sure what is the best way of checking if something is a string or a path (all types of python paths). Any idea?

I could use try/except for Path(value), but wanted to avoid.

[djarecka]

just ignored what i said, we are using try/except block anyway in this function (for reason which I mentioned - didn't know how to check if something is a string or a path), so @effigies suggestions is perfectly fine, will use if isinstance(value, str) and value == ""