Skip to content
This repository has been archived by the owner on Oct 23, 2022. It is now read-only.

BrianGun/issue239 #240

Merged
merged 4 commits into from
Jul 20, 2021
Merged

BrianGun/issue239 #240

merged 4 commits into from
Jul 20, 2021

Conversation

BrianGun
Copy link
Contributor

@BrianGun BrianGun commented Jul 17, 2021

Pull Request Template

Description

Changed Vec3 and Vec4 from FieldVector structs to synonyms for SVector{3},SVector{4}. Vec3,Vec4 were FieldVectors so the x,y,z,w fields could access the entries in the vector. But none of our code uses these accessor fields and having a special type that is equivalent to SVector was causing compatibility issues with with external packages.

While none of our code uses the accessors it is possible that other users have so this could be a breaking change.

Fixes #239

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

passes all unit tests

Checklist:

  • [x ] My code follows the style guidelines of this project
  • [ x] I have performed a self-review of my own code
  • [ x] I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • [x ] My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • [ x] New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

@codecov-commenter
Copy link

codecov-commenter commented Jul 17, 2021

Codecov Report

Merging #240 (cfda04f) into main (465625d) will increase coverage by 3.04%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #240      +/-   ##
==========================================
+ Coverage   51.71%   54.75%   +3.04%     
==========================================
  Files          69       69              
  Lines        6503     6485      -18     
==========================================
+ Hits         3363     3551     +188     
+ Misses       3140     2934     -206     
Impacted Files Coverage Δ
src/Geometry/Transform.jl 49.24% <0.00%> (-2.88%) ⬇️
src/Geometry/Primitives/Curves/Bezier.jl 54.87% <0.00%> (+0.60%) ⬆️
src/Geometry/Primitives/NonCSG/Triangle.jl 74.64% <0.00%> (+1.40%) ⬆️
src/Geometry/CSG/Interval.jl 83.20% <0.00%> (+1.86%) ⬆️
src/Geometry/AccelSurface.jl 96.59% <0.00%> (+2.04%) ⬆️
src/Geometry/Surface.jl 68.18% <0.00%> (+4.54%) ⬆️
src/Vis/Visualization.jl 7.37% <0.00%> (+7.37%) ⬆️
src/Geometry/Primitives/Cylinder.jl 87.03% <0.00%> (+7.40%) ⬆️
src/Geometry/Primitives/Sphere.jl 92.45% <0.00%> (+7.54%) ⬆️
... and 1 more

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 465625d...cfda04f. Read the comment docs.

@alfredclwong
Copy link
Collaborator

@BrianGun how does this fix issue #239?

@BrianGun
Copy link
Contributor Author

it doesn't fix #239. I was using Issues and Pull Requests in VSCode to create the issues and PR's and somehow the numbering got messed up. It's completely independent of the Paraxial analysis stuff.

@BrianGun BrianGun requested review from alfredclwong and galran July 19, 2021 20:07
@BrianGun BrianGun enabled auto-merge (squash) July 19, 2021 20:10
@BrianGun BrianGun removed the request for review from alfredclwong July 20, 2021 21:50
@BrianGun BrianGun merged commit 7a4976a into main Jul 20, 2021
@BrianGun BrianGun deleted the BrianGun/issue239 branch July 20, 2021 22:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tools for doing paraxial analysis
4 participants