Add notes to docs about get_metadata() vs. get_raw_metadata()
#1398
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1398 +/- ##
==========================================
+ Coverage 88.51% 88.59% +0.07%
==========================================
Files 77 77
Lines 10492 10492
==========================================
+ Hits 9287 9295 +8
+ Misses 1205 1197 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
dandi/dandiapi.py
Outdated
| try: | ||
| return models.Dandiset.parse_obj(self.get_raw_metadata()) | ||
| except ValidationError as e: | ||
| raise ValueError( |
There was a problem hiding this comment.
IMHO we should not adapt into another type of exception here -- ValidationError has .errors() etc...
could you please subclass it locally with __init__ to copy over errors and model and adjust its __str__ to get that note ?
There was a problem hiding this comment.
While subclassing looks like it would work in Pydantic 1.x, it won't work in 2.x, as there ValidationError is implemented in Rust, and the only public Python method I see for creating a ValidationError is a static method that can only ever return a ValidationError, not a custom subclass.
There was a problem hiding this comment.
oh! I have missed **v1** in that pydantic/v1/error_wrappers.py path... what could possibly go wrong by mixing Python and Rust? I guess we are yet to see and for now just enjoy public classes/interfaces like
In [5]: ValidationError?
Init signature: ValidationError(self, /, *args, **kwargs)
Docstring: Inappropriate argument value (of correct type).
File: ~/venvs/dev3.11/lib/python3.11/site-packages/pydantic_core/_pydantic_core.cpython-311-x86_64-linux-gnu.so
Type: type
Subclasses:
which cannot even
In [6]: ValidationError()
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
Cell In[6], line 1
----> 1 ValidationError()
TypeError: No constructor defined
heh... I really do not see then a way we could improve UX/DX here then... we should not issue warnings etc. I guess let's just leave the note in the doc string and be done with it.
jwodder
changed the title
get_metadata() vs. get_raw_metadata()get_metadata() vs. get_raw_metadata()
jwodder
added
documentation
|
Thank you @jwodder |
|
🚀 PR was released in |
Closes #1363.