Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow subclassing esm_datastore #415

Closed
aulemahal opened this issue Dec 14, 2021 · 2 comments · Fixed by #417
Closed

Allow subclassing esm_datastore #415

aulemahal opened this issue Dec 14, 2021 · 2 comments · Fixed by #417

Comments

@aulemahal
Copy link
Contributor

Is your feature request related to a problem? Please describe.
In the workflow I am building, which takes the form a package, I want to extend the esm_datastore class with some quite specific functionalities. Mostly things that don't really make sense in intake_esm directly. However, when subclassing esm_datastore, a call to search returns a esm_datastore object because the class is explicitly initialized.

Describe the solution you'd like
Replacing:

cat = esm_datastore({'esmcat': self.esmcat.dict(), 'df': esmcat_results})

with:

        cat = self.__class__({'esmcat': self.esmcat.dict(), 'df': esmcat_results})

Describe alternatives you've considered
A complicated override of search on my subclass. Since the introduction of the derived variable registry, this has been complicated and it's not only a matter of reconstructing a new object with the new df.

@andersy005
Copy link
Member

A complicated override of search on my subclass. Since the introduction of the derived variable registry, this has been complicated and it's not only a matter of reconstructing a new object with the new df.

Sorry to hear about this complication introduced by the DerivedVariable registry functionality... The DerivedVariable registry functionality is a little finicky/fragile for the time being. So, I think your proposed change is reasonable and would welcome a PR.

@aulemahal
Copy link
Contributor Author

I really like the DerivedVariable feature, so I didn't mean this as a critic! The complexity it introduces simply justifies the suggested change. PR coming real soon.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants