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

Return error if argument to parse() is a path to a non-existing file #76

Open
manulera opened this issue Mar 18, 2022 · 4 comments
Open

Comments

@manulera
Copy link
Collaborator

Hello Bjorn,

First of all, thanks for making pydna. I think it's a great library, and it is easy to follow the logic of functions and classes.

I am using it to make a web API, if you want to have a look: ShareYourCloning. At some point I would love to discuss with you about the project.

I just had some bug in my code that took me a while to find. I had mistyped the name of a file, so the parser would return an empty sequence. I thought there was a problem with the file. The thing is that in parsers.py the function parse will not return an error if one passes a path that does not exist. As you say, it is a greedy function and it tries to read the path as a sequence.

I have looked a bit on how to check if a string could be a path, and I didn't find any simple one-liner. I guess the easiest fix to make it back-compatible would be add an extra optional argument input_is_path or something like that default None:

  • None, do what it used to do
  • True try to read the file
  • False try to parse

I guess the case where one would pass both paths and sequences is not very realistic. Let me know if that would be a possible fix and I can make a pull request with the fix and a test.

@BjornFJohansson
Copy link
Owner

Hi, and thanks for this input. As you say, parse is a very greedy function, perhaps too much so.
What do you think of splitting it up into several parse_from_x functions?

for example:

  • parse_from_string()
  • parse_from_path()

This way, the code might be easier to read.

@manulera
Copy link
Collaborator Author

Sounds good, not sure what the best practice is in a case like this. For back-compatibility I guess it's good to keep the function parse, and call parse_from_string or parse_from_path inside parse?

I think it could be entirely removed, but then it would not be back-compatible.

@BjornFJohansson
Copy link
Owner

I would like to keep parse for now, I have lots of old code depending on it.
It could be something that calls parse_from_string and then parse_from_path.

@BjornFJohansson
Copy link
Owner

@manulera I think parse_from_string could be replaces by simply using Dseqrecord("string")?

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