Relax version detection #189
Conversation
Some writers (like for the DANDI project) have stored the versions as floating point values (e.g. `0.2`). Though this won't be long-term sustainable, relaxing the version identification at this point seems unproblematic.
|
I've updated those files to "0.4" as a string. My bad. Hopefully I am the only one. |
Codecov Report
@@ Coverage Diff @@
## master #189 +/- ##
==========================================
+ Coverage 83.99% 84.00% +0.01%
==========================================
Files 12 12
Lines 1362 1388 +26
==========================================
+ Hits 1144 1166 +22
- Misses 218 222 +4
Continue to review full report at Codecov.
|
|
|
||
| # Support floating-point versions like `0.2` | ||
| if isinstance(version, float): | ||
| version = str(version) |
There was a problem hiding this comment.
While this is primarily consumed at the reader level currently and hence quite downstream of the generation process, this raises the question of whether this library should also raise a WARN level logging statement indicating the mismatch.
There was a problem hiding this comment.
Possibly, though I'm already annoyed by the format mismatch warnings. Perhaps we could differentiate between a more purely user (view()) and developer (validate()) context.
There was a problem hiding this comment.
I am quite annoyed by the format mismatch warning as well - happy to create an issue or even look into it 😄
In this scenario, the the warning is about a core issue with the data validation rather than exposing the internal guessing logic of the library so assuming I had to pick between the two, I would personally only keep the former. The idea of differentiating the usage contexts is interesting and possibly brings us back to the validation flag that @will-moore introduced while working on #189.
There was a problem hiding this comment.
I've found why it's duplicated and will open a PR (#190). We'll still need to decide on the first mismatch.
|
LGTM 👍 |
|
Thanks, @will-moore. Anyone have an opinion on the warning? Or shall I just record the fact (in code or an issue) that we may want to warn here in the future when there's a clear "dev" situation? |
|
As per the JSON schema, my assumption is that these datasets should throw a warning with the If so, no objection to getting this in its current form and the warning should naturally be appear as part of the validation framework. |
Some writers (like for the DANDI project) have stored
the versions as floating point values (e.g.
0.2).Though this won't be long-term sustainable, relaxing
the version identification at this point seems
unproblematic.
cc: @LeeKamentsky