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

Major refactor #169

Closed
12 tasks done
sashakames opened this issue Dec 11, 2020 · 4 comments
Closed
12 tasks done

Major refactor #169

sashakames opened this issue Dec 11, 2020 · 4 comments
Assignees

Comments

@sashakames
Copy link
Contributor

sashakames commented Dec 11, 2020

This is an epic issue, we may decompose into smaller tasks:

  • mk_dataset refactor: The functionality in this module is
    • Decompose the functionality into smaller pieces.
    1. express these pieces as generic functions
    2. implement the specific functions
    • create classes of mk_dataset for various projects
    • support custom project DRS loaded from the config file
  • pub_internal refactor
    • make this an object with generic functions
    • implement classes for projects. the class methods call mk_dataset and other modules.
  • Make an architecture diagram for this design.
  • comprehensive test suite to exercise options, maybe automate, some unit testing

Minor details

  • setting/option for verify=True/False
  • cleanup output, make consistent: Error, WARNING, INFO, etc for what gets output in the 3 modes.
  • refactor init_() and use super().init() for common attributes.
@lisi-w
Copy link
Collaborator

lisi-w commented Jan 22, 2021

pub_internal has been refactored for a more object oriented approach, currently works for cmip6, future projects will be added once we finalize design
code for prototype version can be found in the refactor branch

@sashakames
Copy link
Contributor Author

sashakames commented Feb 3, 2021

example code for the purpose of discussion:
https://github.com/lisi-w/esg-publisher/blob/refactor/pkg/esgcet/cmip6.py

  • This is a great start so we can see all the elements
  • I'd suggest we need a base class from which cmip6 is derived.
  • Some key functionality shared across projects should be in that base.
    • The project specific workflow goes in the project class.
    • Pub internal would call a general run() method but implemented by the project class
  • Lets implement a "test" class with the netCDF example (next we can test pure generic data)
    • There was already a "test" project in the old publisher - it had a 3-part drs. we should change that to ... product will determine the test data type
    • the generic (use text file) data case will be helpful. it sorta moves the goalposts but not too much

@sashakames
Copy link
Contributor Author

Adding one more condition: can we refactor init() methods to push the common attributes up to super classes?

@lisi-w
Copy link
Collaborator

lisi-w commented Jun 16, 2021

Further publisher features/testing are covered in other cards/issues. The publisher is fully refactored, but we can reopen this if the need arises.

@lisi-w lisi-w closed this as completed Jun 16, 2021
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

No branches or pull requests

2 participants