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

Make ChiantiPy an Astropy affiliated package #11

Closed
wtbarnes opened this issue Jun 4, 2016 · 22 comments
Closed

Make ChiantiPy an Astropy affiliated package #11

wtbarnes opened this issue Jun 4, 2016 · 22 comments

Comments

@wtbarnes
Copy link
Member

wtbarnes commented Jun 4, 2016

Astropy provides a fairly straightforward template for building Astropy-like Python packages. SunPy currently uses a version of this template.

They provide bootstrapping for package setup and a lot of helpers for building documentation with Sphinx.

Another big advantage is Astropy's units system as well as the many constants that are builtin.

I think standardizing things like the docs and setup will help to preserve the longevity of ChiantiPy as well as encourage future development of the project. Incorporating it into the Astropy family will also make integration with SunPy easier. This will become important as the solar physics community (hopefully) becomes more Python-centric.

@kdere
Copy link
Contributor

kdere commented Jun 13, 2016

Well, i just have to take your word for this as I have not looked very deeply into the structure of the Astropy package. What I saw did look complicated. ChiantiPy does have its own set of constants so I am a bit reluctant to change that.

My reason for putting ChiantiPy on github was to, as you say, to preserve its longevity and encourage future development. In that respect, I am very grateful for your help.

Right now I spend a lot of time fixed bugs, some of which I have recently inserted. Also trying to work on the CHIANTI database and figure out git/github. Haven't figured out how to make a release on github yet.

So, if you could propose a step by step approach to making an acceptable package for astropy/sunpy, that would be great. Needless to say, I will probably need help.

@wtbarnes
Copy link
Member Author

I've not created a release myself via GitHub, but it seems that they have some pretty nice tools for doing just this.

Part of the reason I began looking into the Astropy tools is that they provide a lot of Sphinx helper functions for creating nice documentation with little effort. For example, one can easily document an entire module using .. automodapi:: package.module. Doing so in vanilla Sphinx requires considerably more effort.

One nice example where Astropy could extend the functionality of ChiantiPy is through the use of Astropy tables. For example, the ASCII table produced by fe14.intensityList(wvlRange=[200,220], index=10) could be rendered instead as an Astropy table which shows up as a formatted html table in a Jupyter notebook and can be optionally printed as an AASTeX deluxetable.

I'm happy to help as much as possible with the integration of these features.

I've begun some work implementing the Astropy package template into ChiantiPy. I'll create a pull request to show the changes I've made and outline what I've done so far as a possible step-by-step approach.

@kdere
Copy link
Contributor

kdere commented Jun 14, 2016

thanks for all your help. I will take a look at this asap but I am busy today. but it looks reasonable

@wtbarnes
Copy link
Member Author

wtbarnes commented Jul 9, 2016

Copying over this checklist from merged PR #12

Possible Roadmap for Astropy Integration

An incomplete list of TODOs in order to make ChiantiPy an Astropy affiliated package.

  • Implement basic package template (implemented in PR Make ChiantiPy an Astropy affiliated package #12 )
  • Format doc strings for nice looking Sphinx/RTD docs consistent with Astropy/NumPy guidelines (WIP)
  • Astropy units for all quantities returned from ChiantiPy/calculations within ChiantiPy
  • Astropy tables for outputting spectral line information
  • Test coverage
  • Automatic builds via Travis CI/Appveyor

@kdere
Copy link
Contributor

kdere commented Jul 11, 2016

First, I would like to bring my fork up to compatibility with the main copy. I had made some important edits in some of the files and I think it is good for me to be familiar with the recent changes.
The doc string step look good, but as I said, I think it is necessary to wait a bit for me to catch up.
Astropy units will require a lot of thinking and work.
As for the rest of the step, they look logical but I am unfamiliar with most of them. I have heard of tests/testing but that is about it

@wtbarnes
Copy link
Member Author

Good idea. Better to get your branch even with the main master branch before any other big changes. I didn't mean to rush anything with this checklist. I just wanted to copy it over from the old PR because I thought it more relevant to this issue.

This is of course just a rough outline of possible improvements. Others might have better/more detailed ideas. In general, I think SunPy and Astropy are good examples for documentation, packaging, testing, etc.

@ehsteve
Copy link

ehsteve commented Jul 12, 2016

I'd like to mention that it might be more appropriate for ChiantiPy to become a SunPy affiliated package 👍

@wtbarnes
Copy link
Member Author

Is there an official checklist for SunPy affiliation like there is with Astropy? Or is it essentially the same?

@Cadair
Copy link
Contributor

Cadair commented Jul 13, 2016

https://github.com/sunpy/sunpy-SEP/blob/master/SEP-0004.md#requirements-for-affiliated-packages

@kdere
Copy link
Contributor

kdere commented Jul 13, 2016

Affiliation with SunPy appears to be easier to do. I does not mention anything about changing physical units. The units that ChiantiPy uses are largely compatible with various solar spectroscopic and broadband instruments.

@Cadair
Copy link
Contributor

Cadair commented Jul 13, 2016

@kdere the physical units used are not as much of a problem. It would be nice if the inputs and outputs were Astropy Quantity objects.

@kdere
Copy link
Contributor

kdere commented Jul 14, 2016

I have just brought my fork up to the point where it looks pretty much like the upstream master. So, I created a pull request and it came back that the pull request had a number of incompatibilities that had to sort out.
I looked at file changes and the file .gitignore. There were about 8 red lines at the top which I assume are incompatible. However, these lines are neither in my fork or the upstream-master so I am confused!
Any suggestions

@wtbarnes
Copy link
Member Author

wtbarnes commented Jul 14, 2016

Perhaps try just copying the .gitignore from the upstream (i.e. chianti-atomic/ChiantiPy) into your fork and see if that helps

@kdere
Copy link
Contributor

kdere commented Jul 14, 2016

that is what I did. maybe when I looked at the file edits, The top looks like the astropy .gitignore although not like the SunPy version.
I copied an awful lot of files from the upstream, using the zip file you can download. I thought it would be easiest if everything looked alike. I will look into this some more.

@kdere
Copy link
Contributor

kdere commented Jul 14, 2016

OK, I just did an upstream pull and I will try to get things sorted. io.py is different because I deleted

import sys
as sys is no longer used. will see what the rest consist of.

@kdere
Copy link
Contributor

kdere commented Jul 17, 2016

things are sorted out for now

@kdere kdere closed this as completed Jul 17, 2016
@wtbarnes
Copy link
Member Author

I would suggest reopening this issue and keeping it open until either a) most of the steps for Astropy/SunPy affiliation have been completed or b) the decision is made to not have affiliation with either.

@kdere
Copy link
Contributor

kdere commented Jul 18, 2016

OK, did thtat

@kdere
Copy link
Contributor

kdere commented Aug 24, 2016

is there a way to install astropy as a Python 3 package?

@wtbarnes
Copy link
Member Author

Astropy is compatible with Python 2.7, 3.3, 3.4, and 3.5. From what I understand, there is one codebase and it is compatible with all four versions.

So if you're using Python 3 on your machine, it' just a matter of installing it with conda or pip. For example, to create a new Python 3 environment in conda and install astropy,

$ conda create --name my_new_astropy_env python=3
$ conda install astropy

@wtbarnes
Copy link
Member Author

wtbarnes commented Apr 7, 2017

This should probably be changed to "Make ChiantiPy a SunPy Affiliated package." Details regarding that are here.

It would probably be a good idea to move long term goals like this one to a roadmap in the wiki or something like that...

@wtbarnes
Copy link
Member Author

Closing this as I've moved this issue to a project idea in the wiki as part of an effort to transition longstanding/large scope issues and feature requests to the wiki.

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

No branches or pull requests

4 participants