EdinburghGenomics / pyclarity-lims

Python interface to the GenoLogics LIMS server via its REST API.
MIT License
11 stars 4 forks source link

Issue with queued artifacts #21

Closed Rubyj closed 6 years ago

Rubyj commented 6 years ago

When I try to create a queue on version 0.4.2 of this package I am receiving an error. The queue object is created successfully, however when artifacts and queued_artifacts are accessed a stack trace is returned. The stack trace reports "ValueError: unconverted data remains: -0500". I have a feeling this is to due to the queue time for my sample being something like "2018-01-09T15:44:48.571-05:00".

Thank you

Rubyj commented 6 years ago

After i did a little detective work, it appears that you are doing a split on '+' to remove the timezone in python 2.7. However, in my case the timezone exists with a '-'.

tcezard commented 6 years ago

I would really advise you to switch to python3 because I'm actually striping the timezone for python2.7. Plus I don't usually run python2.7 so I might not catch any of this issues early

Rubyj commented 6 years ago

Unfortunately that is not an option on our current server :/ If you allow me to push a branch I could create a merge request for this issue. Or I can fork the repo.

Some thoughts - the time could be returned as a string for the user to parse. The idea of this from my end was to have the time more easily accessible.

tcezard commented 6 years ago

You can push a branch to you local fork and open a pull request from there

tcezard commented 6 years ago

Quick fix would be to replace

qt = qt.split('+')[0]
m = re.match('(.*)[+-]\d{1,2}\d{1,2}$', qt)
if m:
    qt = m.group(1)

I could leave the string as is but since it is meant to parse I though I would go all the way and parse the time as well. I'll have another look at python2.7 parsing of timezone next week unless you submit a PR before that

Rubyj commented 6 years ago

Yup, I get where you are coming from, but if it causes issues to parse because of python versions etc. Then I feel like it should be on the user to parse in their own way. I will put a PR up on my fork today. Also, side note, using dateparser and pytz could potentially get around this issue.

Rubyj commented 6 years ago

@tcezard can you push out a new version with this fix? Thanks!