pydna icon indicating copy to clipboard operation
pydna copied to clipboard

Add optional argument to parse to determine whether input is path or file content

Open manulera opened this issue 2 years ago • 5 comments

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.

manulera avatar Mar 18 '22 18:03 manulera

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.

BjornFJohansson avatar Jun 16 '22 10:06 BjornFJohansson

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.

manulera avatar Jun 16 '22 14:06 manulera

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 avatar Jun 16 '22 15:06 BjornFJohansson

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

BjornFJohansson avatar Feb 14 '23 09:02 BjornFJohansson