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

Path are coerced to string #287

Open
slaperche-scality opened this issue Mar 24, 2019 · 4 comments
Open

Path are coerced to string #287

slaperche-scality opened this issue Mar 24, 2019 · 4 comments

Comments

@slaperche-scality
Copy link
Contributor

slaperche-scality commented Mar 24, 2019

The documentation mention that doit accept Path object inside file_dep and targets but I noticed that when I got the item back they're no longer Path object but have been converted to string.

e.g:

#!/usr/bin/env python
# coding: utf-8

import pathlib

def task_path():
    """test"""
    def mkdir(targets):
        for directory in targets:
            directory.mkdir()

    return {
        'actions': [mkdir],
        'targets': [pathlib.Path('baz')],
        'clean': True,
    }

gives

% doit path
.  path
TaskError - taskid:path
PythonAction Error
Traceback (most recent call last):
  File "/home/sylvain/dev/doit_bug/partialfunc/venv/lib/python3.7/site-packages/doit/action.py", line 424, in execute
    returned_value = self.py_callable(*self.args, **kwargs)
  File "/home/sylvain/dev/doit_bug/order/dodo.py", line 25, in mkdir
    directory.mkdir()
AttributeError: 'str' object has no attribute 'mkdir'

I would have expected the types to be preserved.

Is it a bug or do you have some internal constraints that force you to convert everything to string?

Fund with Polar
@schettino72
Copy link
Member

Some places expect it to be a string...

When I added support for Path, I did the easy way just converting it when the task is created.

Since you can easily re-create a Path. I am not sure it is worth to implement this. I would need to check the code in details to see everywhere a change is required.

@slaperche-scality
Copy link
Contributor Author

I see, it makes sense.

It could be an issue if you have a lot of paths, then the cost of recreating all the Path objects may be noticeable (that being said, that's not an issue I have for now).

If it's not worth implementing, documenting the behavior to make it less surprising can be enough I think.

@schettino72
Copy link
Member

@slaperche-scality off-topic:

Saw your work on metalk8s, awesome! Also glad you had only 3 (small) requests 😀
I would really appreciate if you could contribute a success story [1] describing your project, how doit is used, and why doit was chosen.

[1] http://pydoit.org/stories.html

@slaperche-scality
Copy link
Contributor Author

No problem 🙂 , I'll submit a PR for that :) (probably next week).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants