poliastro / czml3

Python 3 library to write CZML
https://pypi.org/project/czml3/
MIT License
40 stars 33 forks source link

Revert "make PolylineArrowMaterial() actually work" #80

Closed astrojuanlu closed 3 years ago

astrojuanlu commented 3 years ago

Reverts poliastro/czml3#77.

According to https://github.com/AnalyticalGraphicsInc/czml-writer/issues/181#issuecomment-745598944, we got this one wrong.

The reason might be that we are conflating Polyline, PolylineMaterial and PolylineArrowMaterial, and also mixing the property names with the types.

In particular:

Color is also correct here. Materials do not contain other materials.

See this fragment for an example:

https://github.com/lyqh-ctx/cesiumTx/blob/f6788936274f23c5b1d53671b25c1bcdc78db6e4/examples/czml_polyline.html#L81-L100

cc @mhaberler

codecov-io commented 3 years ago

Codecov Report

Merging #80 (038a2b1) into master (778906b) will not change coverage. The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #80   +/-   ##
=======================================
  Coverage   99.19%   99.19%           
=======================================
  Files          12       12           
  Lines         745      745           
=======================================
  Hits          739      739           
  Misses          6        6           
Impacted Files Coverage Δ
src/czml3/properties.py 100.00% <100.00%> (ø)

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 778906b...038a2b1. Read the comment docs.

astrojuanlu commented 3 years ago

@mhaberler can you check if this snippet works?

Polyline(
    material=PolylineMaterial(
        polylineArrow=PolylineArrowMaterial(
            color=Color(rgba=[200, 100, 30, 255])
        )
    )
)
mhaberler commented 3 years ago

well it does, try here:

from czml3 import Document, Packet, Preamble
from czml3.properties import (
    Color,
    Polyline,
    PolylineMaterial,
    PolylineArrowMaterial,
    PositionList,
)

preamble = Preamble(name="test case")
pl = Polyline(
    material=PolylineMaterial(
        polylineArrow=PolylineArrowMaterial(
            color=Color(rgba=(200, 0, 100, 255))
        )
    ),
    width=10,
    positions=PositionList(cartographicDegrees=[15, 47, 3000, 16, 45, 4000]))
packet = Packet(id="explore https://github.com/poliastro/czml3/pull/80",
                polyline=pl)

print(repr(Document([preamble,packet])))

not sure how I bungled this one up, sorry about that

go ahead and revert, please

astrojuanlu commented 3 years ago

No fuss! You "burgled this up" but I approved and merged, so it's a collective little mistake :) CZML is hard, I'm happy that you are pushing the limits of czml3 and lending a hand with validating all of this!

Merging this 🚀

mhaberler commented 3 years ago

appreciated. In my age I need to substitute fu by persistence ;-)