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

DOC: Document observance for Scientific Python min supported versions #3012

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

jhlegarreta
Copy link
Contributor

Document observance for Scientific Python minimum supported versions recommendation.

Copy link
Member

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

Hi @jhlegarreta,

Thank you for this. We talk about it in https://github.com/dipy/dipy/blob/master/doc/devel/toolchain.rst, So maybe you can also add a reference to this internal document.

@skoudoro
Copy link
Member

What do you think ?

@jhlegarreta
Copy link
Contributor Author

Wasn't aware of that bit. Thanks for doing this. A few comments:

  • To me, this information does not only apply to developers, but to users as well (i.e. those trying to use it through a pip install), so maybe the location of the file should be rethought. Also, I've seen the term "toolchain" be used in other context, like what you would use to compile a C program, so it is misleading to me to use it to include dependencies.
  • Have not gone through the entire document, but for example, I am not sure the table in https://github.com/dipy/dipy/blame/master/doc/devel/toolchain.rst#L50 is clear: what does that line mean: that Python 3.8+, including Python 3.8, is required starting mid-2023? When was 3.7 effectively dropped? We were about to drop 3.8 CI a few weeks ago, so I am not sure about all these timelines. Also, how they overlap with the Scientific Python ecosystem timelines? If they only reproduce them, I would try to redirect to these tables to avoid the risk of going out of sync.

Once the above details get sorted out, I could rebase this PR to cross-reference this document.

@skoudoro
Copy link
Member

To me, this information does not only apply to developers, but to users as well (i.e. those trying to use it through a pip install), so maybe the location of the file should be rethought.

I see what you mean. With your current PR, I think it can stay where it is. They seems to be complementary.

Also, I've seen the term "toolchain" be used in other context, like what you would use to compile a C program, so it is misleading to me to use it to include dependencies.

What is your naming proposition? Currently nothing else come to my mind. I will look at this renaming for next release.

Have not gone through the entire document, but for example, I am not sure the table in https://github.com/dipy/dipy/blame/master/doc/devel/toolchain.rst#L50 is clear: what does that line mean: that Python 3.8+, including Python 3.8, is required starting mid-2023?

starting mid-2023, DIPY support python 3.8+

When was 3.7 effectively dropped?

mid-2023

We were about to drop 3.8 CI a few weeks ago, so I am not sure about all these timelines. Also, how they overlap with the Scientific Python ecosystem timelines? If they only reproduce them, I would try to redirect to these tables to avoid the risk of going out of sync.

the timeline is currently based on the previous releases (existing releases). I have checked all the previous release compatibility. From 2024, we will try to follow scientific-python recommendation. They are recommendation, so we will do our best to follow them but we allow ourselves to make some modification and not be strict on them. for each releases, we would like to support 4 python versions.

@jhlegarreta
Copy link
Contributor Author

What is your naming proposition?

"dipy_ecosystem"? Have not thought enough, but "toolchain" is misleading I'd say.

starting mid-2023, DIPY support python 3.8+

Python 3.8 has been supported since a long time ago, and hence my question.

mid-2023

👍

@skoudoro
Copy link
Member

Python 3.8 has been supported since a long time ago, and hence my question.

Yes, and previous line implied it by saying
mid-2020 Py3.6+
2022 Py3.7+

so the line means: starting mid-2023, DIPY support python 3.8+ or do not support python 3.7 or less

the table show the python minimal supported version

But I see your confusion, Maybe it is better to put in the table when we dropped a python version

@jhlegarreta
Copy link
Contributor Author

Re #3012 (comment) I see: maybe it was my viewpoint the biased one. Maybe it is enough if it is clarified when referring to the paragraph; if this is the usual way this is put, maybe it is not necessary. Thanks.

Copy link

codecov bot commented Dec 16, 2023

Codecov Report

Merging #3012 (bdc9f78) into master (7ff3bf1) will not change coverage.
The diff coverage is n/a.

❗ Current head bdc9f78 differs from pull request most recent head 20649ad. Consider uploading reports for the commit 20649ad to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #3012   +/-   ##
=======================================
  Coverage   82.08%   82.08%           
=======================================
  Files         146      146           
  Lines       20450    20450           
  Branches     3259     3259           
=======================================
  Hits        16786    16786           
  Misses       2855     2855           
  Partials      809      809           

Document observance for Scientific Python minimum supported versions
recommendation.
@jhlegarreta
Copy link
Contributor Author

@skoudoro 20649ad cross-references the document you contributed in commit 3616798.

@jhlegarreta
Copy link
Contributor Author

The coverage build failure is unrelated to this patch set.

README.rst Show resolved Hide resolved
@skoudoro skoudoro merged commit 6b68427 into dipy:master Dec 19, 2023
28 of 29 checks passed
@skoudoro
Copy link
Member

LGTM, merging!

@jhlegarreta jhlegarreta deleted the DocScientificPyVersObserv branch December 19, 2023 19:00
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.

None yet

2 participants