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

Explicitly namespace dim expressions #4320

Merged
merged 20 commits into from
Mar 24, 2020
Merged

Explicitly namespace dim expressions #4320

merged 20 commits into from
Mar 24, 2020

Conversation

philippjfr
Copy link
Member

@philippjfr philippjfr commented Mar 24, 2020

In previous PRs we allowed dim expressions to provide access to any method or accessor. This provides zero validation against typos and is also quite brittle. Instead we have decided that we will provide two additional dim objects to explicitly use specific namespaces, the default dim effectively mirrors the NumPy namespace, while df_dim provides access to the DataFrame-like namespaces, and xr_dim to provide access to the XArray namespace. Instead of use these directly we will allow users to access the namespace by using dim(...).df and dim(...).xr. By default these will only work specifically on objects that can be coerced to conform to this namespace, i.e. a df_dim will only work with columnar (or geometry) data, an xr_dim will work only on gridded data. The df_dim can serve this double purpose since coercion can convert any geometry dataset to either spatialpandas or geopandas which do conform to the dataframe API.

The benefits of namespacing the objects are:

  • Tab-completion of the API
  • Validation of the API (no more typos accidentally making it through)
  • Clear semantics
  • Ability to use namespace on compatible objects through coercion

@jlstevens
Copy link
Contributor

I think this is a very important distinction to make that will help avoid confusion. As I understand it, dim will support methods/behaviors regardless of the underlying data format while the other two will indicate how they are specialized. I would just like to propose pdim and xdim as potential names for the sake of brevity. Your suggestions are more explicit but also a bit more clunky to use...

@philippjfr
Copy link
Member Author

Not a fan of pdim or xdim, I don't think df_dim or xr_dim is too clunky but I'd be okay ommitting the underscore for dfdim and xrdim.

@jbednar
Copy link
Member

jbednar commented Mar 24, 2020

I think the problem with pdim and xdim is that they look like vdims and kdims, i.e., seem to be about new types of dimensions an element can have. Omitting the underscore seems fine.

@jlstevens
Copy link
Contributor

I think the problem with pdim and xdim is that they look like vdims and kdims

Good point. Sounds like Philipp's names without the underscore is our preferred choice right now.

@jbednar
Copy link
Member

jbednar commented Mar 24, 2020

How about dim().pd. ( or dim().df. ) and dim().xr.? I think the semantics of that are clearer than dfdim and xrdim, i.e., it's about accessing the dim() as a Pandas or an Xarray object, or about making a Pandas or an Xarray call on that object.

@jlstevens
Copy link
Contributor

I think I briefly suggested something like dim().df and dim().xr in the discussion yesterday. I'm ok with this (and might slightly prefer it in fact). Up to Philipp!

examples/user_guide/11-Transforming_Elements.ipynb Outdated Show resolved Hide resolved
examples/user_guide/11-Transforming_Elements.ipynb Outdated Show resolved Hide resolved
examples/user_guide/11-Transforming_Elements.ipynb Outdated Show resolved Hide resolved
philippjfr and others added 2 commits March 24, 2020 22:04
Co-Authored-By: James A. Bednar <jbednar@users.noreply.github.com>
@jlstevens
Copy link
Contributor

Looks good to me. I've directly given @philippjfr a few minor items of feedback to consider but otherwise I am very happy with this change!

@jlstevens
Copy link
Contributor

Travis push build passed and pr build only failed due to a transient. Very happy with this PR and it was very worthwhile to get into this release! Merging.

@jlstevens jlstevens merged commit c1894b7 into master Mar 24, 2020
@philippjfr philippjfr deleted the namespaced_transforms branch April 25, 2022 14:41
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.

None yet

3 participants