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 receiver_id to obspy inventory #110

Merged
merged 5 commits into from
Aug 18, 2017
Merged

add receiver_id to obspy inventory #110

merged 5 commits into from
Aug 18, 2017

Conversation

derick-hess
Copy link
Contributor

adresses #107

@derick-hess
Copy link
Contributor Author

If this is good should I go ahead and update the CHANGELOG in this PR as well to reflect the changes we are going to put into 4.0.2?

@derick-hess derick-hess added this to the v4.0.2 milestone Aug 18, 2017
@nick-falco
Copy link
Collaborator

nick-falco commented Aug 18, 2017

Yeah update the change log. I assigned milestones to all the issues resolved in v4.0.2. It's a good idea to reference the issue number for each item.

https://github.com/PIC-IRIS/PH5/milestone/1

I'll test this now.

Copy link
Collaborator

@nick-falco nick-falco left a comment

Choose a reason for hiding this comment

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

The receiver-id shows up at the station level in the StationXML output. I thought receiver-id was the same as the das? Wouldn't this mean that it should be at the channel level?

CHANGELOG.txt Outdated
* Fixed memory leak bug and reduced memory usage
-ph5.clients.ph5tostationxml
-ph5.clients.ph5tostationxml (issue #107)
* added receiver-id to obspy inventory and added an extra atribute at teh station level
Copy link
Collaborator

Choose a reason for hiding this comment

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

@derick-hess Oops I notices a few small typos "teh" instead of "the" and "atribute" instead of "attribute"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll fix that

@derick-hess
Copy link
Contributor Author

Yeah I think channel level might be the more correct place to put that since there are cases you might have multiple receivers at the same station. I'll update the change log and move it to the channel level now

@nick-falco
Copy link
Collaborator

Yeah I think so assuming I'm understanding its meaning correctly. Like you said, there may be cases where you would have 3 single component instruments.

@derick-hess
Copy link
Contributor Author

We do that sometimes with texans where 3 single channel texans are attached to one 3 channel sensor, so having receiver-id at channel level is correct.

Copy link
Collaborator

@nick-falco nick-falco left a comment

Choose a reason for hiding this comment

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

Looks good. Go ahead and squash merge if you're happy with it.

@derick-hess derick-hess merged commit f53a609 into master Aug 18, 2017
@derick-hess derick-hess deleted the fix#107 branch August 18, 2017 21:03
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