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

Coerce some integer data types #108

Closed
wants to merge 1 commit into from

Conversation

amotl
Copy link
Member

@amotl amotl commented Jul 6, 2020

This is just a first shot at #107, further elaboration might touch or resolve aspects from #109.

@codecov
Copy link

codecov bot commented Jul 6, 2020

Codecov Report

Merging #108 into master will increase coverage by 0.11%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #108      +/-   ##
==========================================
+ Coverage   84.07%   84.19%   +0.11%     
==========================================
  Files          32       32              
  Lines         804      810       +6     
  Branches       85       87       +2     
==========================================
+ Hits          676      682       +6     
  Misses        103      103              
  Partials       25       25              
Flag Coverage Δ
#unittests 84.19% <100.00%> (+0.11%) ⬆️
Impacted Files Coverage Δ
wetterdienst/additionals/functions.py 84.61% <100.00%> (+1.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3eea1b3...ff2c36d. Read the comment docs.

@amotl amotl force-pushed the fix-types branch 2 times, most recently from dc8aaf8 to 6e6a4f0 Compare July 6, 2020 17:03
@amotl amotl marked this pull request as ready for review July 13, 2020 20:08
@amotl
Copy link
Member Author

amotl commented Jul 13, 2020

Dear @gutzbenj,

thanks for improving the column type casting mentioned within #109 through your most recent contributions. I've rebased the branch of this PR on top of the current master branch.

Cheers,
Andreas.

@amotl amotl linked an issue Jul 13, 2020 that may be closed by this pull request
@gutzbenj
Copy link
Member

gutzbenj commented Jul 13, 2020

Dear Andreas,

I would rather integrate the integer types in the main function of type coercion. Also with the new column names class-enumeration-mixture arriving through #113 we should fix all those columns at once. I also put comments behind those columns which I believe are integers or strings.

All the best,
Benjamin

@amotl
Copy link
Member Author

amotl commented Jul 14, 2020

Dear Benjamin,

thanks for your excellent work on #113. I will rebase my branch on top of your improvements as soon as they have been merged and will follow your suggestions by integrating the integer type casting into the coerce_column_types() function.

With kind regards,
Andreas.

@gutzbenj
Copy link
Member

Dear Andreas,

as we have already spoken about it, please also include those string data types in the type casting. Those rarely seen columns may hold information about the measuring type of e.g. cloud coverage.

@gutzbenj gutzbenj closed this Jul 22, 2020
@gutzbenj
Copy link
Member

#115

@amotl amotl deleted the fix-types branch September 23, 2020 00:32
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.

Data type anomaly for specific fields within daily data
2 participants