Skip to content

Conversation

sydney-runkle
Copy link
Contributor

  • Remove initial_metadata parameter from build_metadata_dict that encouraged abuse of the metadata construct by magic string injection
  • Improve docstring of CoreMetadataHandler class with TODO relating to next steps for a refactor (I'm happy to investigate this for v2.10)

I investigated removing the metadata property from CoreMetadataHandler due to the abundance of shared logic with __init__ in the same class, but it appears that this is necessary due to the mutation of metadata on the schema passed in. I'll note, in terms of best practices, this should certainly be changed with the aforementioned refactor - the less in place mutation we can do for core schemas, the better.

@github-actions github-actions bot added the relnotes-fix Used for bugfixes. label Aug 20, 2024
@sydney-runkle
Copy link
Contributor Author

I'll note, the docstring for CoreMetadata could most certainly be improved, but it's honestly hard to tell exactly what's going on with the first two fields (specifically, the differences between the two). I might defer this until the refactor when we can hopefully remove these :)

@sydney-runkle sydney-runkle added relnotes-change Used for changes to existing functionality which don't have a better categorization. and removed relnotes-fix Used for bugfixes. labels Aug 20, 2024
Copy link

codspeed-hq bot commented Aug 20, 2024

CodSpeed Performance Report

Merging #10194 will not alter performance

Comparing simplify-metadata (e6507a8) with main (8cd7557)

Summary

✅ 17 untouched benchmarks

Copy link
Contributor

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  pydantic/_internal
  _core_metadata.py
Project Total  

This report was generated by python-coverage-comment-action

@sydney-runkle sydney-runkle merged commit ada82d5 into main Aug 21, 2024
62 checks passed
@sydney-runkle sydney-runkle deleted the simplify-metadata branch August 21, 2024 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes-change Used for changes to existing functionality which don't have a better categorization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants