update Number class to handle integer values

[dave-connors-3]

resolves #8153

Problem

Agate tables, which power dbt show, and therefore the dbt cloud IDE, show integer values as decimals, leading to user confusion when columns seem to have decimals when they don't! this overrides that behavior in the dbt-core customization of the agate Number class

Solution

Ensure that the Number.cast() and Number.jsonify() methods properly handle integer values.

Checklist

• I have read the contributing guide and understand what's expected of me
• I have run this code in development and it appears to resolve the stated issue
• This PR includes tests, or tests are not required/relevant for this PR
• This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX

[github-actions]

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

[codecov]

Codecov Report

Merging #8306 (1644628) into main (991618d) will increase coverage by 0.07%.
Report is 31 commits behind head on main.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #8306      +/-   ##
==========================================
+ Coverage   86.23%   86.30%   +0.07%     
==========================================
  Files         174      174              
  Lines       25518    25575      +57     
==========================================
+ Hits        22005    22073      +68     
+ Misses       3513     3502      -11     

Flag	Coverage Δ
integration	83.08% <100.00%> (?)
unit	65.16% <85.71%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
core/dbt/clients/agate_helper.py	97.54% <100.00%> (+1.01%)	⬆️

... and 21 files with indirect coverage changes

[jtcohen6]

Thanks for putting up the PR @dave-connors-3!

The CI failure reminded me that my recommendation was incomplete.

We need to choose whether we want to cast to int sooner or later:

• Later: just during JSON-ification
• Sooner: as in this approach, but taken one step further to a custom Integer type

I left a comment on the issue with two potential approaches: #8153 (comment)

[emmyoop]

@dave-connors-3 I have capacity this sprint to help get this over this finish line! I agree with @jtcohen6's recommendation to add a new Integer data type, as laid out in his comment. It's the right solution, even with the risk of being a breaking change. Let me know if you'd like some help or even if you don't have capacity right now and I can help get this in.

[dave-connors-3]

thanks @emmyoop!! just pushed jerco's updates -- will test locally shortly, but would love your eyes on it whenever you have a chance!

[dave-connors-3]

@emmyoop -- just got the test_agate_helper.py` tests to pass locally! I am wondering if the other tests should have been updated to assert that they were instances of the custom Integer class as well, but updating them caused local failures. Let me know what you think!

[emmyoop]

@dave-connors-3 your instincts are right that we should change the other checks as well! It's currently failing when you change them to check if they're an instance of agate_helper.Integer because the tables are being generated with agate.Table. The tables that generates is not aware of our custom types. We should update the tests to use agate_helper.table_from_rows instead as that's what we're using elsewhere in the code to account for our custom types. When you make that change, you'll be able to then check for the correct type.

So generally

t1 = agate.Table([(1, "a", None), (2, "b", None)], ("a", "b", "c"))
assert isinstance(result.column_types[0], agate.data_types.Number)

can become

t1 = agate_helper.table_from_rows([(1, "a", None), (2, "b", None)], ("a", "b", "c"))
assert isinstance(result.column_types[0], agate_helper.Integer)

[dave-connors-3]

@emmyoop this one wanted to stay a Number -- guessing it has something to do with the fact that the column is null, but wasn't positive!

[dave-connors-3]

pun not intended -- I wasn't positive, not the number lol

[dave-connors-3]

i also seem to be cursed by the codecoverage bots nvm they like me now

[emmyoop]

This looks great! Thanks so much for getting this in @dave-connors-3!