DASDAE / dascore

A python library for distributed fiber optic sensing
Other
71 stars 16 forks source link

Map plot #385

Closed aissah closed 2 months ago

aissah commented 3 months ago

Description

I added "plot_map", a visualization function that plots the layout of a cable colored by a particular parameter.

Checklist

I have (if applicable):

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.54%. Comparing base (c2f6ca7) to head (d603999). Report is 3 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #385 +/- ## ======================================= Coverage 99.54% 99.54% ======================================= Files 97 98 +1 Lines 7633 7700 +67 ======================================= + Hits 7598 7665 +67 Misses 35 35 ``` | [Flag](https://app.codecov.io/gh/DASDAE/dascore/pull/385/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DASDAE) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/DASDAE/dascore/pull/385/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DASDAE) | `99.54% <100.00%> (+<0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=DASDAE#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

d-chambers commented 3 months ago

Nice work @aissah, it looks very configurable. Do you mind posting an example of the output here so we can see what it looks like?

aissah commented 3 months ago

plot_map_img

Can be reproduced with:

import dascore as dc
patch = dc.get_example_patch("random_patch_with_lat_lon")
patch = patch.set_units(latitude="ft", longitude="m")
ax = patch.viz.plot_map("latitude", "longitude", "distance")

How's this?

d-chambers commented 3 months ago

Looks great!

d-chambers commented 3 months ago

Except, why is latitude in ft and longitude in m?

ahmadtourei commented 3 months ago

A quick suggestion: since we don't use plot in viz's modules (wiggle, waterfall, and spectrogram), I suggest using something like map_fiber instead to make it consistent. What do you think?

aissah commented 3 months ago

Except, why is latitude in ft and longitude in m?

This was an arbitrary choice 😅 illustrating that they can have some units. Would you recommend I change it?

A quick suggestion: since we don't use plot in viz's modules (wiggle, waterfall, and spectrogram), I suggest using something like map_fiber instead to make it consistent. What do you think?

I think this is a good idea. What do you think @d-chambers ?

d-chambers commented 2 months ago

I think this is a good idea. What do you think @d-chambers

Ya, that sounds like a better name to me!

d-chambers commented 2 months ago

This was an arbitrary choice 😅 illustrating that they can have some units. Would you recommend I change it?

Nope, that's perfect. I just wanted to make sure it will use the correct units when we do an actual plot.

aissah commented 2 months ago

I think this is a good idea. What do you think @d-chambers

Ya, that sounds like a better name to me!

Done! I think we might be ready to merge after the checks