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

Define broadcastable for scalars #177

Merged
merged 1 commit into from
Jan 21, 2019
Merged

Define broadcastable for scalars #177

merged 1 commit into from
Jan 21, 2019

Conversation

omus
Copy link
Member

@omus omus commented Jan 18, 2019

Note: AbstractDateTime already had this defined.

Fixes: #175

@omus omus force-pushed the cv/broadcastable branch from f4d03df to 1c7e8f8 Compare January 18, 2019 20:08
@codecov-io
Copy link

codecov-io commented Jan 18, 2019

Codecov Report

Merging #177 into master will decrease coverage by 0.08%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #177      +/-   ##
==========================================
- Coverage   92.05%   91.96%   -0.09%     
==========================================
  Files          30       30              
  Lines        1057     1058       +1     
==========================================
  Hits          973      973              
- Misses         84       85       +1
Impacted Files Coverage Δ
src/types/timezone.jl 95% <0%> (-5%) ⬇️

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 c0071d7...b8e5eea. Read the comment docs.

@omus omus requested a review from iamed2 January 18, 2019 20:09
@omus omus force-pushed the cv/broadcastable branch from 1c7e8f8 to d1be0d3 Compare January 21, 2019 14:47
@iamed2
Copy link
Member

iamed2 commented Jan 21, 2019

The thing about these tests is that they'd still pass if broadcasting was enabled if you defined iteration for time zones.

@omus
Copy link
Member Author

omus commented Jan 21, 2019

The thing about these tests is that they'd still pass if broadcasting was enabled if you defined iteration for time zones.

Would an explicit broadcastable test in addition to the current test be better?

@iamed2
Copy link
Member

iamed2 commented Jan 21, 2019

I think you should just broadcast with something else and test the result

@omus omus force-pushed the cv/broadcastable branch from d1be0d3 to b8e5eea Compare January 21, 2019 17:21
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.

3 participants