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

Helpful declarative __setattr__ and trait validation #2016

Open
3 tasks
dcamron opened this issue Aug 9, 2021 · 8 comments
Open
3 tasks

Helpful declarative __setattr__ and trait validation #2016

dcamron opened this issue Aug 9, 2021 · 8 comments
Labels
Area: Plots Pertains to producing plots Type: Enhancement Enhancement to existing functionality
Milestone

Comments

@dcamron
Copy link
Member

dcamron commented Aug 9, 2021

As it was ripped out of #1903 for now, it might be helpful to users to

@dcamron dcamron added Type: Enhancement Enhancement to existing functionality Area: Plots Pertains to producing plots labels Aug 9, 2021
@dopplershift dopplershift added this to the 1.2.0 milestone Aug 9, 2021
@sgdecker
Copy link
Contributor

Does this issue address the problem described in this notebook?

https://gist.github.com/sgdecker/8c90f2300da03de296e1d934b47e1cd1

@dcamron
Copy link
Member Author

dcamron commented Aug 12, 2021

Does this issue address the problem described in this notebook?

https://gist.github.com/sgdecker/8c90f2300da03de296e1d934b47e1cd1

Accomplishing these tasks should ideally a.) tell you that 'COUNTRIES' is invalid, and b.) tell you so when you attempt to set the trait as opposed to when the figure is drawn. So, "partial yes".

I don't know that this will address the plot parent issue you're having in that notebook. In the mean time it does look like re-creating the PlotObs in the cell above (re-run cells 4-5 instead of just 5) can fix this for now, a full restart/run all not necessary. I'll see if I can boil down where this is happening and open a new issue for it, unless you already have a small reproducible code snippet and want to do that yourself!

@sgdecker
Copy link
Contributor

That notebook is the smallest snippet I have, so by all means, go ahead and open a new issue. Thanks!

@dopplershift
Copy link
Member

dopplershift commented Aug 16, 2021

Here's a minimal reproducer:

from datetime import datetime
from metpy.cbook import get_test_data
from metpy.io import parse_metar_file
from metpy.plots.declarative import PlotObs, MapPanel, PanelContainer

sfc_obs = parse_metar_file(get_test_data('2020010600_sao.wmo', False))

obs = PlotObs()
obs.data = sfc_obs
obs.time = datetime.utcnow()

panel = MapPanel()
panel.projection = 'lcc'
panel.layers = ['countries','states']
panel.plots = [obs]

try:
    pc = PanelContainer()
    pc.panels = [panel]
    pc.show()
except Exception:
    pass

panel = MapPanel()
panel.projection = 'lcc'
panel.plots = [obs]

The call to show() is critical to getting the failure. I'm sure it's some kind of imperfect state management going on here...

@sgdecker
Copy link
Contributor

Let me know if this is worth a new issue, and I realize the existing inconsistencies are just holdovers from the underlying Cartopy feature names, but a list like
panel.layers = ['coastline', 'borders', 'states']
is maddeningly inconsistent.

Why is coastline singular? borders is vague; why isn't it countries?

My request would be to add aliases ('coastlines' = 'coastline', 'countries' = 'borders', and perhaps others).

@dopplershift
Copy link
Member

@sgdecker Can you open that as a new feature request? It's reasonable to do for the declarative interface.

@sgdecker
Copy link
Contributor

sgdecker commented Feb 9, 2024

Please let me know if this should be a separate issue, but a student was flummoxed when the following code excerpt:

pc = PanelContainer()
pc.panel = [panel1, panel, panel3, panel2]
pc.size = (15,12)
pc.show()

generated the error
TraitError: The 'panels' trait of a PanelContainer instance contains an Instance of a List which expected a Panel, not the list [<metpy.plots.MapPanel object at 0x7fd1cace3520>, <metpy.plots.MapPanel object at 0x7fd1cad4cc10>, <metpy.plots.MapPanel object at 0x7fd1cabe8700>, <metpy.plots.MapPanel object at 0x7fd228abd340>].

While a savvy debugger can spot the problem pretty quickly, it isn't entirely obvious (especially when there are many lines of code before this). I believe the panel.setter in the PanelContainer class should check that val is indeed a singular panel.

@dopplershift
Copy link
Member

@sgdecker Yeah, we could probably just check that what's passed in passes isinstance(val, Panel) since that's just a helper property and not a Traitlet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Plots Pertains to producing plots Type: Enhancement Enhancement to existing functionality
Projects
Status: No status
Development

No branches or pull requests

3 participants