Skip to content

adding dataplots.jl#385

Merged
ptiede merged 14 commits intoptiede:mainfrom
erandichavez:main
Feb 1, 2025
Merged

adding dataplots.jl#385
ptiede merged 14 commits intoptiede:mainfrom
erandichavez:main

Conversation

@erandichavez
Copy link
Contributor

Comrade plotting functionality for observation data. Currently supports Makie figures and Makie subplots.

@ptiede
Copy link
Owner

ptiede commented Dec 23, 2024

This looks fantastic and I am really excited to get these in Comrade. The biggest thing that needs to change before merging is moving this to an extension. Other than that the rest of the comments are minor.

There is a potential generalization I mention but maybe we can work on that after this is merged.

@ptiede
Copy link
Owner

ptiede commented Jan 9, 2025

@erandichavez
Copy link
Contributor Author

The plotting code has now been moved to an extension with all your suggestions implemented! (if I blew up your email with these commits i am so sorry)

@ptiede
Copy link
Owner

ptiede commented Jan 30, 2025

Amazing!

One quick thing is to move from CairoMakie to just Makie in the extension/weakdep. If we do that, the extension will load for any Makie backend including GLMakie and CairoMakie.

@codecov
Copy link

codecov bot commented Jan 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.82%. Comparing base (fa08071) to head (d063634).
Report is 18 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #385   +/-   ##
=======================================
  Coverage   92.82%   92.82%           
=======================================
  Files          29       29           
  Lines        1658     1659    +1     
=======================================
+ Hits         1539     1540    +1     
  Misses        119      119           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ptiede
Copy link
Owner

ptiede commented Jan 30, 2025

The doc failures are just because we need to add the new doc strings to the docs.

@erandichavez
Copy link
Contributor Author

I've added docstrings and changed the dependency from CairoMakie to just Makie so i think it should be good to merge now!

@ptiede
Copy link
Owner

ptiede commented Jan 31, 2025

I don't see the docstrings in docs/src/api.md. Could you add them under a new section ### Plotting

@ptiede
Copy link
Owner

ptiede commented Jan 31, 2025

This should do the trick in api.md

## Plotting

!!! warning
    A user must first load a `Makie` backend, e.g., `CairoMakie` to use this functionality

```@docs
Comrade.plotfields
Comrade.axisfields
Comrade.plotcaltable
```

I'm really excited to get this in because I want to use it for some of my 2021 stuff :p

@erandichavez
Copy link
Contributor Author

Done! Docs added to api.md now lets gooooo!!

@ptiede ptiede merged commit a1df86c into ptiede:main Feb 1, 2025
5 checks passed
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.

2 participants