fix(taps): add argument for nan handling strategy to _flatten_record #2222
Conversation
for more information, see https://pre-commit.ci
|
Fix #2213 |
CodSpeed Performance ReportMerging #2222 will not alter performanceFalling back to comparing Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2222 +/- ##
=======================================
Coverage 88.76% 88.76%
=======================================
Files 54 54
Lines 4769 4772 +3
Branches 928 928
=======================================
+ Hits 4233 4236 +3
Misses 374 374
Partials 162 162 ☔ View full report in Codecov by Sentry. |
singer_sdk/helpers/_flattening.py
Outdated
| @@ -20,6 +20,7 @@ class FlatteningOptions(t.NamedTuple): | |||
| max_level: int | |||
| flattening_enabled: bool = True | |||
| separator: str = DEFAULT_FLATTENING_SEPARATOR | |||
| nan_strategy: t.Literal["fail", "allow"] = "fail" | |||
There was a problem hiding this comment.
We may want to support msgspec for serialization in the near future (#1784), and it defaults to making the values null. Do you think there's a way to make these options future-compatible with msgspec?
There was a problem hiding this comment.
Can you specify a little more details? I am not very familiar with msgspec lib. In simplejson which is currently used there is ignore_nan argument which could be used. I am thinking about adding one more nan_strategy "convert_null" which will set ignore_nan to True. WDYT?
There was a problem hiding this comment.
I am thinking about adding one more nan_strategy "convert_null" which will set
ignore_nantoTrue. WDYT?
@dgawlowsky that makes sense to me!
There was a problem hiding this comment.
@edgarrmondragon
Added this! sorry for the delay!
There was a problem hiding this comment.
Thanks @dgawlowsky! I'll review this soon.
There was a problem hiding this comment.
Hi @edgarrmondragon! Can you share the status of the review? We are using meltano in one of our projects and will be deploying do production soon, hence it will be easier to pull library from official distribution. Thanks in advance!
… into handle-nans-in-flattening
|
What's the state of this? I'm trying to develop a tap where there None-values are expected. Do you see a way around this PR to get it done quickly? |
@larsrinn The biggest blocker in this PR is a missing public setting to toggle this behavior, similar to other built-in ones like I'm curious what's your use case for nan-handling? |
📚 Documentation preview 📚: https://meltano-sdk--2222.org.readthedocs.build/en/2222/