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

move analysis.x3dna to new analysis.legacy module #906

Closed
4 of 5 tasks
orbeckst opened this issue Jul 20, 2016 · 2 comments · Fixed by #987
Closed
4 of 5 tasks

move analysis.x3dna to new analysis.legacy module #906

orbeckst opened this issue Jul 20, 2016 · 2 comments · Fixed by #987

Comments

@orbeckst
Copy link
Member

orbeckst commented Jul 20, 2016

As discussed in #898, we cannot easily write full tests for the MDAnalysis.analysis.x3dna module because we cannot easily install x3dna on travis (and I don't even know the terms of the license — impossible to find on the home page!). In accordance with the Guidelines on Testing in MDAnalysis.analysis I propose to

@richardjgowers
Copy link
Member

So how will the namespace work if I want to use analysis.x3dna in the future? Will I have to import from analysis.legacy instead?

I'm not a huge fan of fragmenting namespace like this - could we instead add a warning on import saying words to the effect of, "This is legacy with no tests, double check your results if you use this code"

@orbeckst
Copy link
Member Author

On 20 Jul, 2016, at 15:15, Richard Gowers [email protected] wrote:

So how will the namespace work if I want to use analysis.x3dna in the future? Will I have to import from analysis.legacy instead?

Yes, that was the idea, as discussed.

from MDAnalysis.analysis.legacy import x3dna

The reason for why at least I favored this solution is because it makes it REALLY clear (semantically) that this package is “use at your own risk” – even before you import it. I like the idea of the user having to go the extra mile to import from legacy – should make them think, and should make clear that these packages are not first class citizens.

I’m not a huge fan of fragmenting namespace like this -

Fair enough (“flat is better than nested”) although in this instance I’d consider it justified.

could we instead add a warning on import saying words to the effect of, “This is legacy with no tests, double check your results if you use this code"

I would ALSO add this warning. But warnings are easily ignored/muted.

Anyway, my $0.02.

Oliver Beckstein * [email protected]
skype: orbeckst * [email protected]

@orbeckst orbeckst self-assigned this Aug 12, 2016
orbeckst added a commit that referenced this issue Sep 15, 2016
- created new MDAnalysis.analysis.legacy package
  - directory
  - docs
- x3dna is now considered legacy code (no/minimal testing, no maintenance)
- see #906 for reasons (mainly because the x3dna license does not allow us
  to just install it for testing on travis, or rather, licensing and access
  to x3dna is unknown/too complicated)
- closes #906
- see #743 on background for legacy code and also https://github.com/MDAnalysis/mdanalysis/wiki/Style-Guide#tests-for-mdanalysisanalysis
- added stub with deprecation warning: until release 1.0, MDAnalysis.analysis.x3dna is still accessible
- added test case that the stub is there
orbeckst added a commit that referenced this issue Sep 15, 2016
- legacy module (#743)
- x3dna is now legacy code (#906)
- updated upcoming release number to 0.16.0

[skip ci]
richardjgowers pushed a commit that referenced this issue Sep 15, 2016
* moved analysis.x3dna to analysis.legacy.x3dna

- created new MDAnalysis.analysis.legacy package
  - directory
  - docs
- x3dna is now considered legacy code (no/minimal testing, no maintenance)
- see #906 for reasons (mainly because the x3dna license does not allow us
  to just install it for testing on travis, or rather, licensing and access
  to x3dna is unknown/too complicated)
- closes #906
- see #743 on background for legacy code and also https://github.com/MDAnalysis/mdanalysis/wiki/Style-Guide#tests-for-mdanalysisanalysis
- added stub with deprecation warning: until release 1.0, MDAnalysis.analysis.x3dna is still accessible
- added test case that the stub is there

* updated CHANGELOG

- legacy module (#743)
- x3dna is now legacy code (#906)
- updated upcoming release number to 0.16.0

[skip ci]

* Removed legacy submodule from coverage
abiedermann pushed a commit to abiedermann/mdanalysis that referenced this issue Jan 5, 2017
* moved analysis.x3dna to analysis.legacy.x3dna

- created new MDAnalysis.analysis.legacy package
  - directory
  - docs
- x3dna is now considered legacy code (no/minimal testing, no maintenance)
- see MDAnalysis#906 for reasons (mainly because the x3dna license does not allow us
  to just install it for testing on travis, or rather, licensing and access
  to x3dna is unknown/too complicated)
- closes MDAnalysis#906
- see MDAnalysis#743 on background for legacy code and also https://github.com/MDAnalysis/mdanalysis/wiki/Style-Guide#tests-for-mdanalysisanalysis
- added stub with deprecation warning: until release 1.0, MDAnalysis.analysis.x3dna is still accessible
- added test case that the stub is there

* updated CHANGELOG

- legacy module (MDAnalysis#743)
- x3dna is now legacy code (MDAnalysis#906)
- updated upcoming release number to 0.16.0

[skip ci]

* Removed legacy submodule from coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants