opencdms-dev / pyopencdms-old

⭐🐍 pyopencdms aims to build a common Python API on top of multiple Climate Data Management Systems (CDMS) that use different underlying database engines
MIT License
4 stars 6 forks source link

Domain models available in pyopencdms without ORM dependencies #88

Closed isedwards closed 1 year ago

isedwards commented 1 year ago

As a developer, I want the OpenCDMS Domain models to be available in the pyopencdms repository and for the models to not have dependencies on specific libraries like SQLAlchemy. This will allow me to choose the most appropriate approach/library myself when I am creating a provider plugin to implement interoperability with a different CDMS.

isedwards commented 1 year ago

Hey team! Please add your planning poker estimate with Zenhub @chinedu117 @david-i-berry @scottylad501

isedwards commented 1 year ago

Tasks

We have some housekeeping tasks before we can start this issue:

After these tasks are complete the /opencdms/models/ directory will be ready for the OpenCDMS domain models.

isedwards commented 1 year ago

Following the approach taken in Cosmin Python Chapter 1, define Domain Models in opencdms/models/cdm.py for:

Create unit tests for acceptance tests for correct implementation of each domain model.

chinedu117 commented 1 year ago

Following the approach taken in Cosmin Python Chapter 1, define Domain Models in opencdms/models/cdm.py for:

  • ObservationType()
  • FeatureType()
  • ObservedProperty()
  • ObservingProcedure()
  • RecordStatus()
  • Stations()
  • Sensors()
  • Observations()
  • Collections()
  • Features()

Create unit tests for acceptance tests for correct implementation of each domain model.

Would it be better if we named the model classes in singular while the table names in plural. For example : Sensor(), Observation() instead of Sensors(), Observations(). My reason being that an instance of the model represents one entity.

isedwards commented 1 year ago

Thank you @chinedu117 - I agree.

This is is the usual convention and is also the approach taken in the book.

@david-i-berry could you use Station(), Sensor() etc.?

chinedu117 commented 1 year ago

@isedwards Would it be better to append the suffix "_id" to model properties which are foreignkeys to another table. This way we would prevent name conflict when we define actual relationship. For example: Here: https://github.com/opencdms/opencdms-data-layer/blob/main/data_model/__init__.py#L116

The observed_property field could be renamed to observed_property_id. This way it becomes clear what the field does and saves the observed_property namespace for actual relationship implementation.

isedwards commented 1 year ago

yes - sounds good - that's also standard practice and they've done the same in the book (see highlighted example in the OrderLine class here) @david-i-berry