Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix DEFAULT_PARSERCONTEXT, speed up parsing. #359

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Zentrik
Copy link

@Zentrik Zentrik commented Jan 4, 2024

Unfortunately this was never updated in #280 to be the correct default, and so has been wrong ever since that pr.

#263 suggest significant improvements, I don't see as big an improvement but still substantial.

Unfortunately this was never updated in JuliaIO#280 to be the correct default, and so has been wrong ever since introduction.
Copy link

codecov bot commented Jan 4, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (48e4b21) 99.35% compared to head (bd341d2) 99.35%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #359   +/-   ##
=======================================
  Coverage   99.35%   99.35%           
=======================================
  Files           7        7           
  Lines         466      466           
=======================================
  Hits          463      463           
  Misses          3        3           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -415,9 +415,9 @@ function unparameterize_type(T::Type)
end

# Workaround for slow dynamic dispatch for creating objects
const DEFAULT_PARSERCONTEXT = ParserContext{Dict{String, Any}, Int64, false, nothing}()
const DEFAULT_PARSERCONTEXT = ParserContext{Dict{String, Any}, Int64, true, nothing}()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we change the default to be a non-strict spec, we will certainty want to update the docs and make a version bump. The Python docs may serve as a guide for communicating this: #280 (comment)

Copy link
Author

@Zentrik Zentrik Jan 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I'm missing something, this is a default in the sense that there is a fast path for it. This change doesn't change the result but is merely a performance improvement. If you look further down you'll see that these are the default parameters for parse.

@Zentrik
Copy link
Author

Zentrik commented Aug 8, 2024

Bump

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants