Repository navigation
fix: apply #default annotation when it comes before #datatype - #732
Open
sarankumar1325 wants to merge 2 commits into
Open
sarankumar1325 wants to merge 2 commits into
sarankumar1325 wants to merge 2 commits into
Conversation
FluxCsvParser applied the #default row right away, but the columns are only created by the #datatype row. When #default came first, the defaults were dropped and empty cells parsed as None. Keep the #default row like #group and apply it when the column names are parsed; reset it for each new table. Closes influxdata#727
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #732 +/- ##
==========================================
+ Coverage 90.49% 90.50% +0.01%
==========================================
Files 40 40
Lines 3568 3572 +4
==========================================
+ Hits 3229 3233 +4
Misses 339 339 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #727
Proposed Changes
FluxCsvParserapplied the#defaultannotation row as soon as it was read, but the table columns are only created by the#datatyperow. If#defaultcame before#datatype,add_default_empty_valueslooped over an empty column list and the defaults were lost, so empty cells were parsed asNone.The parser already keeps
#groupuntil the column-name row, so#defaultnow works the same way: the row is stored in the parser metadata and applied when the column names are parsed. It is reset when a new table starts, so one table's defaults don't carry over to the next. The usual#datatype,#group,#defaultorder gives the same result as before.Tests in
tests/test_FluxCSVParser.py:test_default_annotation_before_datatype: the reproduction from the issue (string and long defaults)test_default_annotation_before_datatype_more_tables: several tables with different annotation orders, and a check that defaults don't leak into a table without#defaultBoth fail on
masterand pass with the change. The rest oftest_FluxCSVParser.pypasses, the httpretty-basedtest_QueryApi.py/test_QueryApiDataFrame.pyresults are the same before and after (the only failures need a running InfluxDB), andflake8pluspydocstyleare clean on the changed file.I used an AI coding assistant for this change and reviewed and tested it myself.
Checklist
pytest testscompletes successfully (unit tests pass; integration tests need a running InfluxDB)