mshumko / asilib

An open source package providing data access and analysis tools for the world's all-sky imager (ASI) data.
https://aurora-asi-lib.readthedocs.io/
GNU General Public License v3.0
10 stars 4 forks source link

Acknowledgements #24

Closed CassandraAuri closed 3 months ago

CassandraAuri commented 4 months ago

PR summary

Added a new kwarg to the trex-rgb, trex-nir and rego imagers, acknowledge_always, which printed the acknowledgement during the loading of every new imager, rather than once a month. This was addressing #23 which had a permissionerror due to streamlit cloud not working with os.write embedded within acknowledge.py

-->

PR checklist

CassandraAuri commented 4 months ago

No idea why the checks failed, when I tested it I ran into no problems.

CassandraAuri commented 3 months ago

This code looks good after you consider my suggested changes. One thing to consider is whether you want to print the acknowledgment every single time you instantiate an Imager object. I can see this getting very repetitive in the terminal if you'd like to make mosaics.

Now that I see the pros and cons of this implementation, here is an alternative implementation that I think will be a good compromise. It is also easy to implement.

Include an acknowledge=True kwarg in REGO and TREx loaders (THEMIS will have one eventually) and remove all calls to the acknowledge() function. With this change, new users will see the acknowledgment statement every single time they create an an Imager instance. Once some users become fed up with their terminal overflowing with these statements, they can read up on our documentation and set acknowledge=False.

This sounds good,

make acknowledge_always -> acknowledge I will set acknowledge=True by default, this will call meta['acknowledge']. Then if the user gets annoyed the acknowledge=False just passes, remove acknowledge.py

mshumko commented 3 months ago

Those steps look correct. I really like this solution because the asilib code will stay simple which I always strive for.

Feel free to make those changes or I can make them as well.

P.S. Another simple idea with acknowledge=True is to print the acknowledgment only once per python session. This is possible since the asilib.config dictionary is created import asilib. We can add a asilib.config['acknowledged']=[] to __init__.py. The first time a loader function is run with acknowledge=True it will print the statement and append to the asilib.config['acknowledged'] list. Subsequent calls to that function will find the ASI name in theasilib.config['acknowledged'] list, and not print the statement.

mshumko commented 3 months ago

The 0.23.2 asilib version with this PR is now on PyPI. Please pip install the new version and see if it works with streamlit.

CassandraAuri commented 3 months ago

Works!