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

Release v1.6 #2251

Closed
14 of 17 tasks
francisco-dlp opened this issue Sep 6, 2019 · 35 comments
Closed
14 of 17 tasks

Release v1.6 #2251

francisco-dlp opened this issue Sep 6, 2019 · 35 comments

Comments

@francisco-dlp
Copy link
Member

francisco-dlp commented Sep 6, 2019

I think that it is time to start planning the next minor release that is going to be the last one of the 1 series i.e. the last one before the split.

The split will make it more difficult to contribute code that you have started developing based on the current HyperSpy structure because chances are that at least part of your code will then belong to a different package. Therefore, I suggest we focus our efforts on finishing and merging as many pending contributions as possible, rather than in making brand new contributions.

Since we will be breaking the API, this is also a good time to streamline the syntax by e.g. pruning methods are no longer relevant, changing the default value of arguments when that leads to a better outcome, renaming functions etc. All those API changes should come with a deprecation warning in v1.6 so that users have time to adapt their scripts. I propose to list and discuss in this issue those API changes in a checklist.

PRs

@jlaehne
Copy link
Contributor

jlaehne commented Sep 6, 2019

I think the support for non linear axis #1927 is a clear candidate for v1.6. I'll be happy to contribute to the implementation.

@jlaehne
Copy link
Contributor

jlaehne commented Sep 6, 2019

The following Components are so far not implemented as Expression. For which does it make sense to aim at a conversion, is it feasible or even desirable for all (some are quite complex, see the mentioned requirements)?

It would also solve the issue that 'function_nd' is not defined for some of these functions, see #2225

@francisco-dlp
Copy link
Member Author

Good point, indeed the best way to fix #2225 is to implement as many components as possible using Expression.

  • arctan: there is no where in there. ifelse could be avoided by splitting the component in 2 e.g. Arctan for the standard function and EELSArctan for the one with minimum_at_zero==True. (The arctan is commonly used in EELS to approximate CL edges, and that's the reason it was added to HyperSpy).
  • heaviside can be implemented using Expression and numpy's heaviside function.
  • Probably the standard voigt function can also be implemented using Expression. The PES part of the code could go into a new PESVoigt function that we could move to a new PES package after the split.
  • eels_cl_edge cannot be implemented using Expression. It is possible to implement a function_nd method for it for the case where the fine structure coefficients are not free and don't change.
  • I don't think that scalable_fixed_pattern can be written as an Expression, but it will be good to add a function_nd method (possible when interpolate is not True)
  • pes_core_lineshape and pes_see are components that I wrote almost 10 years ago to process the data in this paper and they haven't changed much since. At the time I couldn't even dream that my humble scripts in time were going to become HyperSpy, so the code and structure is not up to the current HyperSpy standards. Once we split HyperSpy in 2.0, those components will move to their own PES package, and that will be the time to give them a bit of love. In the meantime I suggest we focus our efforts elsewhere.

@tjof2
Copy link
Contributor

tjof2 commented Sep 12, 2019

I hope #2035 fits the brief of "streamline the syntax" and makes it in

@francisco-dlp
Copy link
Member Author

@tjof2, thanks for bringing #2035 back to our attention. I have created a checklist in the header post and added #2035 to it.

@dnjohnstone
Copy link
Contributor

Could we get #1497 in here too please? I don't really understand how to solve the gui related bits.

Picking up our other conversation about electron diffraction packages - one way to solve issues emerging in the hyperspy community is to upstream this kind of core functionality that's not really got anything to do with diffraction into hyperspy.

@francisco-dlp
Copy link
Member Author

Thanks, I've just added it to the list. For those who don't know which conversation @dnjohnstone is talking about pyxem/pyxem#409

@AEljarrat
Copy link
Contributor

AEljarrat commented Sep 16, 2019

Hi Everyone,

I happen to have already implemented the Voigt lineshape as an Expression for a personal project. It is identical to the original function in hyperspy, but without the parameter estimation and PES specific parameters. You can find the code using this link.

I'm a little out of the loop lately, but I'll be happy to contribute this to hyperspy if its interesting!

Best regards,
Alberto.

@thomasaarholt
Copy link
Contributor

@AEljarrat Check your link? :p

@AEljarrat
Copy link
Contributor

Fixed the link. Thanks @thomasaarholt

@tjof2
Copy link
Contributor

tjof2 commented Apr 12, 2020

Obvious other candidates #2352, #2358, #2203, #2172

@tjof2
Copy link
Contributor

tjof2 commented Apr 24, 2020

@francisco-dlp I also have a large-ish PR to come focusing on #1159 and the rest of the decomposition stuff. Hoping to have it up by the weekend/early next week. Would be nice for that to make it in.

@francisco-dlp
Copy link
Member Author

I think that trying to keep up with the plan of making v1.6 the last release of the 1 series is counterproductive because we are not anywhere near the target of merging most of the open pull requests, while many PRs has been merged since the latest minor release. So, what about releasing in the next few weeks v1.6? Which PRs could we realistically merge in that time span so that they make it into the release?

@ericpre
Copy link
Member

ericpre commented Apr 24, 2020

I agree. I would add a couple of low hanging fruit:

Shall we include the non linear axis branch in the next release too? I think it would be more rather than making a separate beta release.

@jlaehne
Copy link
Contributor

jlaehne commented Apr 27, 2020

+1 for including the non linear axis branch (there were a few more fixes necessary though)

@francisco-dlp
Copy link
Member Author

I suggest we fix a date for the release, otherwise we'll be tempted to keep on delaying it until this or that feature is ready. What about the 15th of May with a feature freeze on the 13th of May?

I agree that it will be good to add the non-linear axes feature if ready. The already implemented features not too far off. There are missing ones that we could leave for later, since the partial support that is already implemented is a big step forward. In my mind, the most important things to get right for this release are: i) saving to file ii) the syntax.

@ericpre
Copy link
Member

ericpre commented Apr 27, 2020

This sounds a very good plan to me!

For the syntax side of non linear axis, it would be good to address #2345 at the same time.

@jlaehne
Copy link
Contributor

jlaehne commented Apr 27, 2020

#2262 should be basically ready to be included, has a first review, but might need another one.

@jlaehne jlaehne mentioned this issue Apr 27, 2020
1 task
@tjof2
Copy link
Contributor

tjof2 commented Apr 27, 2020

#2383 is quite big but I think would be a worthy addition - it improves a lot plus fixes a fair few things that never quite worked.

@tjof2
Copy link
Contributor

tjof2 commented May 14, 2020

#1159 can be checked off.

#2314 would be good - there's currently no good tests for ragged mapping as I found in #2391. I don't know if it fully fixes #2266?

#1757 is low-hanging fruit.

@francisco-dlp
Copy link
Member Author

I suggest delaying the release 1 week. I've been busier than expected with other tasks and I bet that the same is true for several of us.

@jlaehne
Copy link
Contributor

jlaehne commented May 16, 2020

#2184 (Powerlaw naming), #2262 (Heaviside function as expression), #2395 (Arctan/EELSArctan as expression) should be low hanging fruit to include.

@francisco-dlp
Copy link
Member Author

In a current discussion in #2299 some of us are considering further delaying this release until some remaining key issue with the non-uniform axes features are addressed. Are there any argument against further delaying this release?

@tjof2
Copy link
Contributor

tjof2 commented May 19, 2020

@francisco-dlp how many other significant things have been added? 25 days ago you wrote:

many PRs has been merged since the latest minor release

That would be a good thing to compare against waiting for the non-uniform axes.

@francisco-dlp
Copy link
Member Author

That's definitely true. Which are the advantages/disadvantages of delaying the release say by ~1 month?

Advantages from #2299, mainly pointed out by @jlaehne :

  • Take advantage to the momentum of a new release to push the non-uniform features forwards
  • non-uniform axes is a much demanded feature, and we are not too far from getting it in good enough shape for a release, see Non-uniform axes progress tracking #2398

Can somebody think of disadvantages, that can outweight the benefits above? In particular, are there people out there who will be badly impacted by such a delay?

@thomasaarholt
Copy link
Contributor

Can somebody think of disadvantages, that can outweight the benefits above? In particular, are there people out there who will be badly impacted by such a delay?

Asked a different way, what "content" will be provided with 1.6? Perhaps we should write a draft of the release notes (would have to be done anyway with either release date). That would make it clearer what the major pros/cons of delaying are.

@francisco-dlp
Copy link
Member Author

For that we could have a look at https://github.com/hyperspy/hyperspy/milestone/37?closed=1 . Of course updating CHANGES.rst would be better. I've assigned the Rnp PRs to v1.6 because I thought that there wasn't going to be a v1.5.3. However, since we are still discussing when to release v1.6, if we finally go for a delay, we could consider releasing v1.5.3 if identify PRs merged into Rnp that justify releasing asap.

@jlaehne
Copy link
Contributor

jlaehne commented May 19, 2020

Looking at the checklist on the top of this page, other mentions in this thread and the open issues in the milestones (which partly overlap of course), there are a couple of other things that might make it into v1.6 if we delay by a month.
Indeed, there are also a lot of merged PRs in the milestone already but it looks more like patches and smaller features (apart from maybe the improvements concerning decomposition?).

@tjof2
Copy link
Contributor

tjof2 commented May 19, 2020

A cursory glance, some key things already in are absorption correction, serpentine scan pattern, decomposition overhaul (+lots of additions to docs), performance improvements to FFTs across the board, threading control for parallel map, and many other doc improvements.

@francisco-dlp
Copy link
Member Author

I think that there is no question that there are way enough important PRs merged since v1.5 to justify a new release. The main issue is, should we wait 1 month for the non-uniform axes and other PRs to be included in this release? I think that it is worth it unless some communities out there are itching for some of the features already merged and will be badly impacted by the delay.

@francisco-dlp
Copy link
Member Author

@ericpre, is there any reason to keep the freetype < 2.10 constraint for the tests?

@ericpre
Copy link
Member

ericpre commented Aug 3, 2020

freetype=2.9 is used for plot testing, if a different version is used, the plotting test will fail.
At some point, it would be good the reference images to a more recent matplotlib version and it may be good to do it when moving to use constraint_layout in favour of tight_layout (https://matplotlib.org/3.3.0/tutorials/intermediate/constrainedlayout_guide.html).

@francisco-dlp
Copy link
Member Author

Why waiting for using constraint_layout? In other words, if there is nothing else blocking, should we do it for this release?

@ericpre
Copy link
Member

ericpre commented Aug 3, 2020

Yes, it is one option, constainted_layout will be particular useful for plot_images where it would be possible to improve the layout of a single colorbar with multiple images.

@francisco-dlp
Copy link
Member Author

I have just tagged some of the items in the v1.6 milestone with the release_highlight label i.e. to be included in CHANGES.rst. I may have missed some important PRs or tagged some that may not deserve to be highlighted, so please feel free to amend the list.

@tjof2 tjof2 closed this as completed Aug 6, 2020
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

7 participants