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: R encoding of pathlib.Path objects #1201

Merged
merged 7 commits into from Nov 23, 2021

Conversation

jhrcook
Copy link
Contributor

@jhrcook jhrcook commented Oct 1, 2021

Description

Snakemake currently throws the following error if a pathlib.Path object is passed in params (where dir is the name of the directory created with Path(“dir”)):

ValueError: Unsupported value for conversion into R: dir

I have made the following change to the REncoder class to add support explicit for conversion of pathlib.Path objects.

class REncoder:
    """Encoding Pyton data structures into R."""

    ….

    @classmethod
    def encode_value(cls, value):
        if value is None:
            return "NULL"
       ...
        elif isinstance(value, Path):  # Just added these two lines.
            return str(value)
        ...
        raise ValueError("Unsupported value for conversion into R: {}".format(value))

I have added a test that throws an error without this change in place but runs successfully with this fix.

QC

  • The PR contains a test case for the changes or the changes are already covered by an existing test case.
  • The documentation (docs/) is updated to reflect the changes or this is not necessary (e.g. if the change does neither modify the language nor the behavior or functionalities of Snakemake).

@jhrcook jhrcook requested a review from johanneskoester as a code owner Oct 1, 2021
@johanneskoester
Copy link
Contributor

johanneskoester commented Oct 21, 2021

Ah, this was already fixed by a different PR before. Nevertheless, the test case is very useful, so I will keep and merge that. Thanks!

@johanneskoester
Copy link
Contributor

johanneskoester commented Oct 21, 2021

Did you commit the test case? I cannot see it in the diff.

@jhrcook
Copy link
Contributor Author

jhrcook commented Oct 21, 2021

Sorry, I didn’t realize that I needed to force add the test files to git. I have included them in the latest commit.

@sonarcloud
Copy link

sonarcloud bot commented Oct 26, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@johanneskoester johanneskoester merged commit bd516e9 into snakemake:main Nov 23, 2021
6 checks passed
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

Successfully merging this pull request may close these issues.

None yet

2 participants