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

DM-40948: Add TMA eventMaker function for getting a specific event #63

Merged
merged 4 commits into from Sep 29, 2023

Conversation

mfisherlevine
Copy link
Contributor

No description provided.

Repeated calls for the same ``dayObs`` will use the cached data if the
day is in the past, and so will be much quicker. If the ``dayObs`` is
the current day then the EFD will be queried for new data for each
call, so a call which returns ```None` on the first try might return an
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
call, so a call which returns ```None` on the first try might return an
call, so a call which returns ``None`` on the first try might return an

Copy link
Contributor

Choose a reason for hiding this comment

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

And actually, might even just be None I'm fuzzy on the different use cases for single vs double `.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, that was supposed to be a single-backquoted None. Not sure how to invoke that with github markdown...

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 am equally fuzzy 😬 We can certainly both agree that three/one isn't the right format though 😛

nEvents = len(events)

event = eventMaker.getEvent(self.dayObs, 0)
self.assertIsInstance(event, TMAEvent)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if the API supports it, but could you add self.assertEqual(event, events[0]) ?
And similar for events[100]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, it looks like past-Merlin actually did implement __eq__ for these objects, so yes, I'll do that - nice!

@mfisherlevine mfisherlevine merged commit 273ad40 into main Sep 29, 2023
1 check 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.

None yet

2 participants