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

Bundle imageutils for photutils 0.1 release #132

Merged
merged 1 commit into from
Sep 11, 2014

Conversation

astrofrog
Copy link
Member

As discussed in astropy/imageutils#15, for the photutils 0.1 release we should bundle imageutils in photutils/extern, because it will not be available in the Astropy core as astropy.image yet.

Technically this can be done by copying over files into photutils/extern or maybe by creating a git submodule in photutils/extern ... to be investigated / discussed here.

cc @larrybradley @astrofrog

@cdeil cdeil added this to the 0.1 milestone Aug 6, 2014
@larrybradley
Copy link
Member

Ideally we would use a submodule, but I don't know if it's possible.

@astrofrog
Copy link
Member

A submodule is possible, so it's more a question of whether people using photutils from git will remember to checkout the submodule.

@eteq
Copy link
Member

eteq commented Aug 6, 2014

Yeah, but this it's a "solved problem" both in IPython and ah_bootstrap by having python setup.py build check the submodule situation. We could do that here...

OTOH, it might not be worth it given that it's only a transient situation that imageutils is a submodule. So it might just be better to do it manually (and also manually stick it into the release tarball), and it should be unnecessary for future releases.

@cdeil
Copy link
Member Author

cdeil commented Aug 6, 2014

I'm 👍 on copying the files into photutils/extern/imageutils, because it's the simpler solution where it's clear that it will just work without having to configure and investigate how a new git submodule interacts with python setup.py.

@eteq
Copy link
Member

eteq commented Aug 8, 2014

@cdeil - you mean just for the release, or actually tracking them in git? The latter is mildly problematic because we'll have to do a lot of updates given the rate of change in imageutils...

@cdeil
Copy link
Member Author

cdeil commented Aug 17, 2014

@cdeil - you mean just for the release, or actually tracking them in git?

Not sure which is better ... I'm fine with any scheme that works.
(I don't think it's too important because this is a temp situation anyways and the problem will be gone in a few months when Astropy 1.0 comes out with astropy.image.)

@eteq
Copy link
Member

eteq commented Aug 18, 2014

@cdeil - I thought the idea is that a lot of stuff that's getting added now in imageutils is needed for photutils, right? So the latter is sort of a "testing ground" for the former? Or are you thinking most of what photutils needs is already there by now?

@cdeil
Copy link
Member Author

cdeil commented Aug 18, 2014

@eteq - Everything photutils needs is already now in imageutils (because everything photutils needs was already available in photutils and we simply moved it to imageutils).
Now while we are re-factoring imageutils we have to adapt photutils ... it's a bit annoying and it would be best to progress quickly on imageutils in the next 10 days before the photutils 0.1 release, but everyone's time is limited.

@astrofrog
Copy link
Member

It turns out that it's easier to include the source files by hand because including it as a submodule means we also have the root of the imageutils directory which we don't want - so unless there are any objections, I will go ahead and merge this (without this, it is not possible to run asv benchmarks since it requires all dependencies to be on pypi, and I think we are not planning on releasing imageutils).

@astrofrog
Copy link
Member

In any case, development on imageutils has slowed down a bit (because what is there is close to converged) so the rapid development is no longer really an issue.

@astrofrog
Copy link
Member

@cdeil or @larrybradley - does this seem ok to you?

@astrofrog
Copy link
Member

Failing because of astropy/imageutils#22

@astrofrog
Copy link
Member

Should be fixed now.

@astrofrog
Copy link
Member

(I there are no objections I will merge this evening since we can always switch to a submodule if we want)

@larrybradley
Copy link
Member

Can astropy/imageutils#17 be merged and included in this PR? I never merged it because I was expecting detailed comments. It may not address the needs of every affiliated package, but it's definitely better (and cleaner API) than what's currently in imageutils master.

@astrofrog
Copy link
Member

@larrybradley - sounds good, I'll go and take a look at that PR

@astrofrog
Copy link
Member

@larrybradley - I've merged astropy/imageutils#17 and have updated this PR.

@eteq
Copy link
Member

eteq commented Sep 11, 2014

@astrofrog - ah, good point about the problem of imageutil's root being the wrong thing... So I guess this works, then.

astrofrog added a commit that referenced this pull request Sep 11, 2014
Bundle imageutils for photutils 0.1 release
@astrofrog astrofrog merged commit ab5e3c3 into astropy:master Sep 11, 2014
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.

4 participants