rstudio / pins-python

https://rstudio.github.io/pins-python/
MIT License
52 stars 12 forks source link

Bugfix: off-by-one error in error message for version parsing #283

Closed nathanjmcdougall closed 3 months ago

nathanjmcdougall commented 3 months ago

Another nitpicky issue with these error messages:

https://github.com/rstudio/pins-python/blob/d215a8cd6cf958be0f4fc41c6e90e21c907b4655/pins/versions.py#L67-L73

If there is a malformed version spam-eggs-spam, this will emit version string can only have 1 '-', but contains 3 Which is misleading, since there are only 2 hyphens, not 3.

While changing this, I think I'd prefer to use try-except ValueError around the unpacking of parts rather than have a dedicated != 2 check so that could be changed too.

isabelizimm commented 3 months ago

Ah, the off by one value error I can't unsee 🙈 I think the !=2 is an okay check, is there a particular reason to make the swap to a ValueError?

nathanjmcdougall commented 3 months ago

It just avoids having to write an explicit check (so I think it makes clearer the connection between the check and the unpacking step that follows), but really it's a style/preference thing so I'll leave it be.