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

Add z_bias_map schema #100

Merged
merged 4 commits into from
Aug 23, 2023
Merged

Add z_bias_map schema #100

merged 4 commits into from
Aug 23, 2023

Conversation

dachengx
Copy link
Contributor

@dachengx dachengx changed the title Add z_bias_map schema Add z_bias_map schema Aug 22, 2023
Copy link
Member

@jmosbacher jmosbacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @dachengx !

Just one thing missing, the fields of the schema :)
Inheriting from TimeIntervalCorrection will add the version: str and time: TimeInterval fields but there is no fields defined to hold the map.
I see now that the maps will be stored in private_nt_aux_files so i guess this schema will only store the file name for each time period? In that case either add a value: str field or inherit from the BaseMap schema which has a value field of type string.
from .base_references import BaseMap, but BaseMap also has the algorithm field which may not be relevant.

Another more advanced option would be to add a field called e.g. filename: str and add a property named value which loads the file when accessed. I didn't do this for the BaseMap class because i was afraid it may confuse people.

from .base_corrections import TimeIntervalCorrection

class ZBias(TimeIntervalCorrection):
_ALIAS = 'z_bias_map'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually we name the collection alias plural, so here it would be z_bias_maps just so it makes more sense when people try to access the data from xedocs.

@dachengx
Copy link
Contributor Author

If add a property value, and we know that the map is in .json.gz, then shall I open the file inside value? I think this is done by a straxen fucntion.

@jmosbacher
Copy link
Member

For simplicity, maybe just make it similar to what we did for all the other maps.

@jmosbacher
Copy link
Member

jmosbacher commented Aug 23, 2023

Have you had a look at the xedocs.schemas.corrections.base_references.BaseResourceReference class?
This class checks that the file exists in MongoDB before inserting it into the DB.
Would this kind of check also be useful for this map?

@dachengx
Copy link
Contributor Author

Have you had a look at the xedocs.schemas.corrections.base_references.BaseResourceReference class? This class checks that the file exists in MongoDB before inserting it into the DB. Would this kind of check also be useful for this map?

Sorry, I misunderstood your point. I think BaseResourceReference is certainly better.

@jmosbacher jmosbacher merged commit 6f00f6d into master Aug 23, 2023
4 checks passed
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.

2 participants