USNavalResearchLaboratory / nepc

NRL Evaluated Plasma Chemistry
Creative Commons Zero v1.0 Universal
13 stars 9 forks source link

Moves CustomCS to CS class #77

Open gracetangg opened 2 years ago

gracetangg commented 2 years ago

I've moved the CustomCS constructor into the CS class constructor.

I also added a flag for whether or not the CS class is customized, however, this is mainly for whether or not to make the "cs_id" = None if neither the metadata or the data is provided. From the description of CustomCS it seems like the user needs at least one of metadata or data if we are building from an existing CS dataset, so in that case we wouldn't need this flag. But in the tests, we can create CustomCS instances without providing either, so in that case we would need the extra flag, so please let me know what you think.

Addresses #12

padamson commented 2 years ago

Thanks for tackling this one, it has been top on my todo list for a while.

I took a stab at adding some tests for exceptions that would be raised to think through how customizing a CS class should work. I'd prefer not to have a flag when calling CS but rather the initializer would recognize whether the user is customizing an existing dataset or creating one from scratch, then raises appropriate exceptions if an illegal combination of cursor, cs_id, data, or metadata are provided.

We could expand the documentation for the CS class to explain what is allowed, and the exception handling will provide additional clarification.

We could have an attribute that is set to indicate whether the CS is builtin (straight out of database), custom (all data/metadata provided when CS was called), or customized (cursor and cs_id provided, and metadata/data provided to over-ride). Not sure what to call the attribute. Perhaps source?

padamson commented 2 years ago

In case it's not clear, here is what I'm thinking for the source attribute: