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

Code refactoring: Replace Sphere by ShellEnvelope, Shape3D heritage, and move everything under geometryObjects #40

Merged
merged 13 commits into from
Jun 14, 2018

Conversation

JulienPeloton
Copy link
Member

Three major things in this PR:

  • Move ShellEnvelope, BoxEnvelope in geometryObjects sub-folder
  • ShellEnvelope and BoxEnvelope herit from Shape3D and no longer Envelope. By doing so, all geometrical objects come from Shape3D
  • Replace Sphere by sub-case of ShellEnvelope (innerRadius=0.0), and completely remove the Sphere class.

There are also minor changes:

  • Shape3D has been simplified. All subclasses have been modified accordingly.
  • For ShellEnvelope and BoxEnvelope I changed the declaration of the center: it is now a val, so cannot be changed or nulled afterwards (tests also modified accordingly).
  • Add two new constructors to ShellEnvelope to easily define a Sphere from the Shell
  • Update intersect method of Point3D with Shell and Box envelopes
  • Add license header for spatialPartitioning/Octree*.scala.
  • A few typos fix here and there... (but probably miss a few)

From the user point of view, nothing really change (loading the data from SphereRDD is unchanged).
However I think it would be a good idea later on to include the possibility to load Shells as well. We could even have only one routine to load Point3D (3 args), Sphere (4 args), and Shells (5 args)... TBD.

@codecov-io
Copy link

codecov-io commented Jun 13, 2018

Codecov Report

Merging #40 into master will increase coverage by 0.11%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #40      +/-   ##
==========================================
+ Coverage   98.87%   98.99%   +0.11%     
==========================================
  Files          19       18       -1     
  Lines         713      697      -16     
  Branches      179      176       -3     
==========================================
- Hits          705      690      -15     
+ Misses          8        7       -1
Impacted Files Coverage Δ
...park3d/spatialPartitioning/OctreePartitioner.scala 100% <ø> (ø) ⬆️
...ark3d/spatialPartitioning/OctreePartitioning.scala 100% <ø> (ø) ⬆️
...in/scala/com/spark3d/geometryObjects/Shape3D.scala 100% <ø> (ø) ⬆️
...in/scala/com/spark3d/spatial3DRDD/Shape3DRDD.scala 96.96% <ø> (ø) ⬆️
...com/spark3d/serialization/Spark3dRegistrator.scala 100% <ø> (ø) ⬆️
...ark3d/spatialPartitioning/SpatialPartitioner.scala 50% <ø> (ø) ⬆️
...park3d/spatialPartitioning/OnionPartitioning.scala 100% <ø> (ø) ⬆️
...spark3d/spatialPartitioning/OnionPartitioner.scala 100% <ø> (ø) ⬆️
...scala/com/spark3d/spatialPartitioning/Octree.scala 100% <ø> (ø) ⬆️
...la/com/spark3d/geometryObjects/ShellEnvelope.scala 97.43% <100%> (ø)
... and 3 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 e2a8210...058e929. Read the comment docs.

@JulienPeloton
Copy link
Member Author

Before merging, I still need to update notebooks.
Once merged, I will bump code version to 0.1.1.

@JulienPeloton
Copy link
Member Author

Notebooks are fine -- nothing changed. Merging and bumping the version.

@JulienPeloton JulienPeloton merged commit 0510df1 into master Jun 14, 2018
@JulienPeloton JulienPeloton deleted the uniformization branch June 14, 2018 08:38
@mayurdb
Copy link
Contributor

mayurdb commented Jun 14, 2018

The code looks much cleaner now :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants