-
Notifications
You must be signed in to change notification settings - Fork 21
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
New package #32
New package #32
Conversation
Some feedback I got:
|
…groupby metadata field when getting dataset ids
This commit solves the issue of mismatch between the nework name in the IsmnFileCollection class and the name in the folders. * The name is now parsed from the folder name * The tests have been updated to check this * A sensor from the network FR_Aqui has been included in the test dataset to check the mismatch FR_Aqui/FR-Aqui
Review new ismn package
at the following stations is 1 sensor that creates an error in the metadata generation: |
I think the metadata issue is due to the csv files. and catching errors is what the reader should do and does in those cases. So this is open for merging again. |
src/ismn/interface.py
Outdated
|
||
@property | ||
def grid(self): | ||
return self.networks.grid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be self.collection.grid
?
yes, good point, changed it.
________________________________
Von: pstradio ***@***.***>
Gesendet: Dienstag, 16. März 2021 09:13
An: TUW-GEO/ismn
Cc: Preimesberger, Wolfgang; Author
Betreff: Re: [TUW-GEO/ismn] New package (#32)
@pstradio commented on this pull request.
________________________________
In src/ismn/interface.py<#32 (comment)>:
+ def __getitem__(self, item):
+ return self.collection[item]
+
+ def __repr__(self):
+ return f"{self.root}\n" \
+ f"with Networks[Stations]:\n" \
+ f"------------------------\n" \
+ f"{self.collection.__repr__(' ')}"
+
+ @Property
+ def networks(self):
+ return self.collection.networks
+
+ @Property
+ def grid(self):
+ return self.networks.grid
Shouldn't this be self.collection.grid ?
-
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub<#32 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AF2HQH2PWRNWHDQQXHZGPGDTD4HLXANCNFSM4WVA2HLA>.
|
Fixed bug in parsing nw name from folder
@daberer thanks for merging, now I'd suggest you check everything again, docs and build. Then when you are happy with everying and everything is ready for a new release (which should be a new major version, i.e. This will trigger when you draft a new release on github (right side releases on the overview page), but before that you have to change the owner name in https://github.com/TUW-GEO/ismn/blob/master/.github/workflows/release.yml#L14 from |
Hi,
so here is the version of the package that I have been working on for some time.
It would be good to get feedback and/or to merge and release this
It's using @sebhahn's classes for stations, networks sensors etc. but also implements the
ISMN_Interface
as before (where metadata is stored and loaded, so that using the ismn data is still fast). This is not fully backward compatible, but I tried to make the new class as similar to the old one as possible, so that people can still use it as before.Here is a list of changes:
Examples on using it are in https://github.com/wpreimes/ismn/blob/master/docs/read_and_plot_ismn_data/interface.ipynb
Things that might dislike compared to the old package:
@daberer @Adeaem @sebhahn any feedback?
Let me know what you think we should change, this is probably not perfect yet, but especially the new metadata handling, relative paths and reading from zip I think is very much needed.