Add support for external custom data #4
Conversation
There was a problem hiding this comment.
Hi @HealthyPear ! Thanks for the implementation!:)
I tried the script, but got some errors as below:
| if not performance_data: | ||
| energy_bins, gamma_rate, background_rate = prepare_data(config) | ||
| else: | ||
| energy_bins, gamma_rate, background_rate = prepare_data(performance_data) |
There was a problem hiding this comment.
Here we need to put both of config and performance_data
src/iact_estimator/core.py
Outdated
| raise NotImplementedError( | ||
| "MAGIC SUM trigger at mid zenith range has not been yet implemented." | ||
| ) | ||
| if ("LST" in performance_metadata) and config["sum_trigger"]: |
There was a problem hiding this comment.
performance_metadata is not assigned when not using performance_data, so I got an error when I launched the script without custom dataset
There was a problem hiding this comment.
correct, this is related to the thread above
docs/source/userguide.rst
Outdated
| # - {name: gamma_rate, datatype: float64} | ||
| # - {name: background_rate, datatype: float64} |
There was a problem hiding this comment.
I think it would be nice to add unit for gamma_rate and background_rate as examples.
There was a problem hiding this comment.
right, when I produced this example table I forgot the units for this columns!
There was a problem hiding this comment.
should be better now, I basically copied the header from the MAGIC low energy data file
|
I have to fix the conflicts before merging...the CLI is now unified under a single entry point |
|
@SeiyaNozaki should be all working now, can you re-review? :) |
This PR adds support for loading data from file built using astropy and saved in ECSV format like the public data shipped with the package.
Documentation has been updated accordingly
@SeiyaNozaki you can test this by installing and switching to this branch; you can compile the new docs locally (see instructions) - let me know if you can replicate (correctly) your previous results
Closes #1 #2