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

Make StationLookup a proper Mapping #2398

Merged
merged 2 commits into from
Apr 5, 2022
Merged

Conversation

dopplershift
Copy link
Member

@dopplershift dopplershift commented Mar 24, 2022

Description Of Changes

This allows iteration, membership, etc. directly on the station information rather than needing to use the tables property. The abstract base class gives us some things like get(), keys(), and values() for free.

Checklist

@dopplershift dopplershift added Type: Enhancement Enhancement to existing functionality Area: IO Pertains to reading data labels Mar 24, 2022
@dopplershift dopplershift added this to the 1.3.0 milestone Mar 24, 2022
@dopplershift dopplershift requested a review from a team as a code owner March 24, 2022 21:34
@dopplershift dopplershift requested review from dcamron and removed request for a team March 24, 2022 21:34
@dopplershift
Copy link
Member Author

@jthielen Do you think it would be enough to have a docstring example of use here? Of course, station_info and StationLookup don't seem to show up in our reference pages atm. Or did you have a more proper gallery example in mind?

@jthielen
Copy link
Collaborator

jthielen commented Mar 24, 2022

@jthielen Do you think it would be enough to have a docstring example of use here? Of course, station_info and StationLookup don't seem to show up in our reference pages atm. Or did you have a more proper gallery example in mind?

Yes and yes? Having StationLookup in the API reference with a docstring example (and description of where this info is from) was my initial thought, but having a gallery example would be nice as well. Given that we have geopandas in the doc environment, for the latter I was thinking a demo loading the station info into a GeoDataFrame and then applying some example spatial operation? I should be able to put something like that together quick tomorrow.

This allows iteration, membership, etc. directly on the station
information rather than needing to use the tables property.
@dopplershift
Copy link
Member Author

Ok, I added StationLookup and station_info to the docs for metpy.io, and I added a brief docstring example.

I'll leave a proper example to later, when I can come up with something useful--or if you get some cycles to breathe life into your idea @jthielen .

Copy link
Member

@dcamron dcamron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice and helpful! Small change for docs render. Also, question: are instances exported with the Exporter context manager supposed to automatically end up with generated doc summaries? If so, I'm not seeing that here after a quick glance.

src/metpy/io/station_data.py Outdated Show resolved Hide resolved
Includes a basic doctestable example.
@dopplershift
Copy link
Member Author

Exporter's primary mission in life is managing __all__. Sphinx doesn't appear to do anything with instances, and so far I haven't found any magic incantations to change that. I suppose we could implement a custom module template. 😬

@dcamron
Copy link
Member

dcamron commented Apr 5, 2022

I think I'm very okay skipping that and calling the explicit shoutout in the class docstring good enough!

@dcamron dcamron merged commit 6408e3a into Unidata:main Apr 5, 2022
@dopplershift dopplershift deleted the stationlookup branch April 5, 2022 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: IO Pertains to reading data Type: Enhancement Enhancement to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add station_info to docs
3 participants