Update following changes made in lstchain on pointings

[labsaha]

Following recent changes in the lstchain repo for drive pointings, we need to update LSTDriveContainer class as well.

[rlopezcoto]

Thanks @labsaha! Is this change just "cosmetic" (to change the name "Altitude" to "Zenith") or is it conflicting with the new fields defined in lstchain because of the pointing changes?

[labsaha]

@rlopezcoto It is just cosmetic, as you mentioned: Altitude to Zenith

[maxnoe]

Please do not do this! At least not without conferring first with CTA, pinging @kosack here.

We use altitude everywhere else. This is not a "cosmetic" change!

[labsaha]

@maxnoe In the drive file the values are given Azimuth and Zenith. Hence to be consistent with that it is done in that way.

[maxnoe]

Then you probably should convert into altitude on reading the input file before filling the container as everything after that, including astropy coordinates, expect altitude, not zenith.

[labsaha]

Well, that can be done. But It should not give the wrong message that in the drive report we have Altitude because we are calling the LSTDriveContainer where all fields are considered following the drive report.

[maxnoe]

IMHO, consistency of the in-memory representations in ctapipe/lstchain would be preferable over consistency with the input files.
This is also how these problems are usually handled best: at I/O boundaries conversions take place and internally everything should be consistent.

But this is just my 2 cents, maybe @kosack or @bregeon can also comment.

I see that just the container is changed here. Is it just defined and not used in this repo?
If it is not used here, why is it here?

[labsaha]

@maxnoe I agree with that we should maintain the consistency. Initially, I thought of converting Zenith to Altitude before filling the container. However, at the same time, I thought about the field which is given in the drive logs. Finally, I have gone for the second one. I didn't realize that it might break the consistency in ctapipe. Anyway, I can close the PR once we hear from @kosack @bregeon. And make associated changes in lstchain

[labsaha]

I see that just the container is changed here. Is it just defined and not used in this repo?
If it is not used here, why is it here?

I cannot comment on this, even to my satisfaction. In the past, at one point it was decided somehow to put this here to include all monitoring information (one of them is drive log) in this container class.

[FrancaCassol]

Hi @maxnoe,

I see that just the container is changed here. Is it just defined and not used in this repo?
If it is not used here, why is it here?

you are right, it could be filled here but somehow we didn't menage to converge toward this solution.

@labsaha I agree with @maxnoe, please keep consistency with ctapipe.

[vuillaut]

I see that just the container is changed here. Is it just defined and not used in this repo?
If it is not used here, why is it here?

I think the idea was that this information should come with the data?

[labsaha]

Closing this PR and make required changes in lstchain