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

Remove deprecation Bruker, EMD, Nexus #30

Merged
merged 8 commits into from Aug 30, 2022
Merged

Conversation

jlaehne
Copy link
Contributor

@jlaehne jlaehne commented Aug 29, 2022

Description of the change

Remove deprecated keywords and attributes from readers before first release (which will be the one used by HyperSpy 2.0, for which these deprecations were announced).

Closes #24

Progress of the PR

  • Change implemented (can be split into several points),
  • update user guide (if appropriate),
  • add an changelog entry in the upcoming_changes folder (see upcoming_changes/README.rst),
  • Check formatting changelog entry in the readthedocs doc build of this PR (link in github checks)
  • remove tests,
  • ready for review.

@codecov
Copy link

codecov bot commented Aug 29, 2022

Codecov Report

Merging #30 (3a42f44) into main (8357de4) will increase coverage by 0.06%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main      #30      +/-   ##
==========================================
+ Coverage   82.67%   82.73%   +0.06%     
==========================================
  Files          40       40              
  Lines        8050     8032      -18     
  Branches     1819     1857      +38     
==========================================
- Hits         6655     6645      -10     
+ Misses        924      920       -4     
+ Partials      471      467       -4     
Impacted Files Coverage Δ
rsciio/bruker/api.py 88.25% <ø> (+0.22%) ⬆️
rsciio/emd/api.py 85.34% <ø> (-0.13%) ⬇️
rsciio/nexus/api.py 91.82% <ø> (-0.17%) ⬇️
rsciio/tvips/api.py 98.47% <0.00%> (+<0.01%) ⬆️
rsciio/phenom/api.py 85.28% <0.00%> (+0.03%) ⬆️
rsciio/usid_hdf5/api.py 91.90% <0.00%> (+1.20%) ⬆️
rsciio/image/api.py 91.56% <0.00%> (+2.40%) ⬆️
rsciio/conftest.py 71.42% <0.00%> (+28.57%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@jlaehne jlaehne added this to the v0.1.0 initial release milestone Aug 29, 2022
CHANGES.rst Outdated
@@ -8,6 +8,20 @@ https://rosettasciio.readthedocs.io/en/latest/changes.html

.. towncrier release notes start

API Removal
-----------
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As this is relevant to hyperpsy users and the API was in hyperspy before, would it be better to move it to the corresponding entry of hyperpsy 2.0 changelog?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, we might want to just duplicate it there, in case migrating HyperSpy users end up looking here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still find odd to have a API removal section for a first release! I would remove this section and a normal changelog entry - the description on its own is good.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved it to a regular towncrier entry for the PR and checked the formatting locally.

rsciio/tests/test_nexus_hdf.py Outdated Show resolved Hide resolved
Copy link
Member

@ericpre ericpre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@ericpre ericpre merged commit 9775138 into hyperspy:main Aug 30, 2022
@jlaehne jlaehne deleted the deprecation branch August 30, 2022 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove deprecated code
2 participants