Menu
▾
▴
docutils-develop
|
From: Adam T. <aa-...@us...> - 2022-02-05 02:00:27
|
Meta -- is there a better way to review/work on these patches than the SF interface? Could I be given access to a `sandbox/blah` folder that we could use for prototyping instead of uploading loads of files here? A --- ** [bugs:#441] Move from "optparse" to "argparse".** **Status:** open **Created:** Thu Jan 06, 2022 03:02 PM UTC by Günter Milde **Last Updated:** Sat Feb 05, 2022 01:44 AM UTC **Owner:** Günter Milde The optparse documentation says: > Deprecated since version 3.2: The optparse module is deprecated and will not be developed further; development will continue with the argparse module. We are currently suppressing related deprecation warnings in the test suite. After raising the Python dependency to >=3.7, now may be the right time to make the move. --- Sent from sourceforge.net because doc...@li... is subscribed to https://sourceforge.net/p/docutils/bugs/ To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/docutils/admin/bugs/options. Or, if this is a mailing list, you can unsubscribe from the mailing list. |
|
From: Adam T. <aa-...@us...> - 2022-02-05 02:35:06
|
High level review: - The header states the GPL licence rather than public domain -- would you be happy to say public domain instead? It would be really bad to add GPL code into the core of Docutils, which is currently public domain / BSD. - Strong negative on validation within ConfigParser. I think we should make validation distinct from parsing, but that should be a different change - I think the "ACTIVE" section could be a method or an `@property` -- `ConfigParser.active_config` or suchlike, that would be accessed by `OptionParser`. - Explicit side effect handling -- this is fine but we should document that only "XXX" is allowed to have side effects for backwards compatability purposes, and ideally we would deprecate the idea of side effects. - apart from that it seems reasonable at a high level, let me know if you want a review of the specific implementation (there's a few things I'd change but I don't want to derail the discussion) A --- ** [bugs:#441] Move from "optparse" to "argparse".** **Status:** open **Created:** Thu Jan 06, 2022 03:02 PM UTC by Günter Milde **Last Updated:** Sat Feb 05, 2022 01:47 AM UTC **Owner:** Günter Milde The optparse documentation says: > Deprecated since version 3.2: The optparse module is deprecated and will not be developed further; development will continue with the argparse module. We are currently suppressing related deprecation warnings in the test suite. After raising the Python dependency to >=3.7, now may be the right time to make the move. --- Sent from sourceforge.net because doc...@li... is subscribed to https://sourceforge.net/p/docutils/bugs/ To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/docutils/admin/bugs/options. Or, if this is a mailing list, you can unsubscribe from the mailing list. |
|
From: Guenter M. <mi...@us...> - 2022-02-05 16:01:44
|
On 2022-02-05, Adam Turner wrote:
> High level review:
> - The header states the GPL licence rather than public domain -- would
> you be happy to say public domain instead?
This is from my standard template for new Python files. For Docutils, I use
the 2-clause BSD for new files and the orignal license for patches to
existing files.
> - Strong negative on validation within ConfigParser. I think we should
> make validation distinct from parsing, but that should be a different
> change
A strong point for validation in the parse stage is, that you can
have per-configfile error messages/warnings. It is suboptimal when an
invalid value (a negative interger, say) could be from any of the
configuration files + command line but the warning/error does not tell.
Per-file warnings are also possible, if the arg_parser calls the
ConfigParser instance in a loop::
for cf in configuration_files:
cf_parser.read(file)
validate(cf_parser['ACTIVE'])
The alternative concept would be a "stand-alone" ConfigParser that would:
* parse SettingsSpecs for active sections, validation functions,
and side-effects
* read config files
* merge active settings from active sections
* convert and validate the values
This way, applications that don't parse the command line would not need to
set up an ArgumentParser instance at all.
For this, the new SettingsSpecs.argument_spec format should be:
* easy to code/read and understand in the component files
* easy to convert into `ArgumentParser.add_argument()` calls.
* easy to parse for active sections, validation functions,
and side-effects in the ConfigParser.
A common utility would convert the current SettingsSpecs.settings_spec
format into the new one.
> - I think the "ACTIVE" section could be a method or an `@property` --
> `ConfigParser.active_config` or suchlike, that would be accessed by
> `OptionParser`.
I would prefer to stay with the common interface from "configparser":
1. set up <instance> (here, also pass info about components)
2. <instance>.read() (this will also merge the settings)
3. Get the data as <instance>[section] (or <instance>.<dict instance attribute>).
> - Explicit side effect handling -- this is fine but we should document
> that only "XXX" is allowed to have side effects for backwards
> compatability purposes, and ideally we would deprecate the idea of
> side effects.
I would keep the additional "overrides" keyword as well as the
"validate_encoding_and_error_handler()" action open for use in
settings-specifications. Maybe discourage its use, though.
> - let me know if you want a review of the specific implementation
> (there's a few things I'd change but I don't want to derail the
> discussion)
A peer-review of the implementation would be welcome. You may send a
patch/link in private mail or just mention me at github to keep the
discussion here more on the high-level aspects.
Thanks, Günter
|
|
From: Günter M. <mi...@us...> - 2022-02-05 22:26:56
|
On 2022-02-05, Adam Turner wrote:
> High level review:
> - The header states the GPL licence rather than public domain […]
This is from my standard template for new Python files. For Docutils, I use
the 2-clause BSD for new files and the orignal permissions for patches to
existing files.
> - Strong negative on validation within ConfigParser. I think we should
> make validation distinct from parsing, but that should be a different
> change
A strong point for validation in the parse stage is, that you can
have per-configfile error messages/warnings. It is suboptimal when an
invalid value (an out-of-bound interger, say) could be from any of the
configuration files or the command line but the warning/error does not tell.
Processing should go ahead, though with the faulty value ignored (maybe it is from a config file inaccessible to the user but overridden in a project or personal config file or on the command line).
Per-file warnings are also possible, if the ArgParser instance calls the
ConfigParser instance in a loop::
for cf in configuration_files:
cf_parser.read(file)
validate(cf_parser['ACTIVE'])
The alternative concept would be a "stand-alone" ConfigParser that:
* parses SettingsSpecs for active sections, validation functions,
and side-effects
* reads config files
* merges active settings from active sections
* converts and validates the values
This way, applications that don't parse the command line would not need to
set up an ArgumentParser instance at all.
For this, the new SettingsSpecs.argument_spec format should be:
* easy to code/read and understand in the component files
* easy to convert into `ArgumentParser.add_argument()` calls.
* easy to parse by the ConfigParser (for active sections, validation functions, and side-effects ).
A common utility would convert the current SettingsSpecs.settings_spec
format into the new one.
> - I think the "ACTIVE" section could be a method or an `@property` --
> `ConfigParser.active_config` or suchlike, that would be accessed by
> `OptionParser`.
I would prefer to stay with the common interface from "configparser":
1. set up <instance> (here, also pass info about components)
2. <instance>.read() (this will also merge the settings)
3. Get the data as <instance>[section] (or <instance>.<dict instance attribute>).
> - Explicit side effect handling -- this is fine but we should document
> that only "XXX" is allowed to have side effects for backwards
> compatability purposes, and ideally we would deprecate the idea of
> side effects.
I would keep the additional "overrides" keyword as well as the
"validate_encoding_and_error_handler()" action open for use in
settings-specifications. Maybe discourage its use, though.
> - let me know if you want a review of the specific implementation
> (there's a few things I'd change but I don't want to derail the
> discussion)
A peer-review of the implementation would be welcome. You may send a
patch/link in private mail or just mention me at github to keep the
discussion here more on the high-level aspects.
Thanks, Günter
---
** [bugs:#441] Move from "optparse" to "argparse".**
**Status:** open
**Created:** Thu Jan 06, 2022 03:02 PM UTC by Günter Milde
**Last Updated:** Sat Feb 05, 2022 02:18 AM UTC
**Owner:** Günter Milde
The optparse documentation says:
> Deprecated since version 3.2: The optparse module is deprecated and will not be developed further; development will continue with the argparse module.
We are currently suppressing related deprecation warnings in the test suite.
After raising the Python dependency to >=3.7, now may be the right time to make the move.
---
Sent from sourceforge.net because doc...@li... is subscribed to https://sourceforge.net/p/docutils/bugs/
To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/docutils/admin/bugs/options. Or, if this is a mailing list, you can unsubscribe from the mailing list. |
|
From: Günter M. <mi...@us...> - 2022-02-08 17:15:14
|
Attached a review and patch proposal. More "high-level" planning and proof-of-concept files under sandbox/enhancement-proposals/settings-API Attachments: - [Add-helper-methods-for-frontend-and-core-review.txt](https://sourceforge.net/p/docutils/bugs/_discuss/thread/c76f9a2618/45c6/3ffa/attachment/Add-helper-methods-for-frontend-and-core-review.txt) (5.8 kB; text/plain) - [argparse-gm.patch](https://sourceforge.net/p/docutils/bugs/_discuss/thread/c76f9a2618/45c6/3ffa/attachment/argparse-gm.patch) (19.1 kB; text/x-patch) --- ** [bugs:#441] Move from "optparse" to "argparse".** **Status:** open **Created:** Thu Jan 06, 2022 03:02 PM UTC by Günter Milde **Last Updated:** Sat Feb 05, 2022 10:26 PM UTC **Owner:** Günter Milde The optparse documentation says: > Deprecated since version 3.2: The optparse module is deprecated and will not be developed further; development will continue with the argparse module. We are currently suppressing related deprecation warnings in the test suite. After raising the Python dependency to >=3.7, now may be the right time to make the move. --- Sent from sourceforge.net because doc...@li... is subscribed to https://sourceforge.net/p/docutils/bugs/ To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/docutils/admin/bugs/options. Or, if this is a mailing list, you can unsubscribe from the mailing list. |
|
From: Adam T. <aa-...@us...> - 2022-02-20 05:53:31
|
Replying to comments from the review file here:
> * Use single quotes for strings (""" for docstrings) (↗policies.txt).
> * Avoid line length > 79 chars.
Yep, will fix
> Missing the "usage" [argument].
Added back, I removed it as it overrides the more helpful message that argparse provides by default but yes there is a remote possibility someone might be using it
> Not needed. Note the "Filenames will be tilde-expanded later." in the docstring.
I moved the tilde expansion to the class constructor for a minor performance boost. What I forgot to do was remove the docstring comment.
> Changed behaviour: Users may set ``$DOCUTILSCONFIG=~/.config/docutils.conf``.
I don't think it is, as the "DOCUTILSCONFIG" in environment path includes tilde expansion. The fast path is only when "DOCUTILSCONFIG" does not exist (likely the majority of cases).
> Place at end of file.
Sure
> We have 3 settings sources: components, config files, command line: * Are settings after updating from config files still "default" settings?
You could argue yes, the settings are the default for the particular environment. Happy to take better suggestions, though.
> I propose a more versatile variant that allows for "settings_overrides" and customization of the config file names.
I see the idea, but I would very strongly advise against -- this is meant to be a simple convinience function. If the more complex functionality is needed later, we can always either add it to the same function, or add a new one.
> I propose a more general helper function ... Publisher.get_components()
This looks good. I would document it as a private / provisional, method, but I think this could be used now.
A
---
** [bugs:#441] Move from "optparse" to "argparse".**
**Status:** open
**Created:** Thu Jan 06, 2022 03:02 PM UTC by Günter Milde
**Last Updated:** Tue Feb 08, 2022 05:15 PM UTC
**Owner:** Günter Milde
The optparse documentation says:
> Deprecated since version 3.2: The optparse module is deprecated and will not be developed further; development will continue with the argparse module.
We are currently suppressing related deprecation warnings in the test suite.
After raising the Python dependency to >=3.7, now may be the right time to make the move.
---
Sent from sourceforge.net because doc...@li... is subscribed to https://sourceforge.net/p/docutils/bugs/
To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/docutils/admin/bugs/options. Or, if this is a mailing list, you can unsubscribe from the mailing list. |
|
From: Adam T. <aa-...@us...> - 2022-02-20 06:14:34
|
I have pushed the changes from the review and Publisher.get_components into https://github.com/AA-Turner/docutils/pull/8 A --- ** [bugs:#441] Move from "optparse" to "argparse".** **Status:** open **Created:** Thu Jan 06, 2022 03:02 PM UTC by Günter Milde **Last Updated:** Sun Feb 20, 2022 05:53 AM UTC **Owner:** Günter Milde The optparse documentation says: > Deprecated since version 3.2: The optparse module is deprecated and will not be developed further; development will continue with the argparse module. We are currently suppressing related deprecation warnings in the test suite. After raising the Python dependency to >=3.7, now may be the right time to make the move. --- Sent from sourceforge.net because doc...@li... is subscribed to https://sourceforge.net/p/docutils/bugs/ To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/docutils/admin/bugs/options. Or, if this is a mailing list, you can unsubscribe from the mailing list. |
|
From: Guenter M. <mi...@us...> - 2022-02-26 11:30:43
|
On 2022-02-20, Adam Turner via Docutils-develop wrote: > I have pushed the changes from the review and Publisher.get_components > into https://github.com/AA-Turner/docutils/pull/8 What is your intention with the GHA scripts? On my system, cd repo/docutils flake8 . works fine with the settings in `docutils/tox.ini`, so duplication as `docutils.flake8` does not seem required. Günter |
|
From: Adam T. <aat...@ou...> - 2022-02-27 00:48:02
|
> What is your intention with the GHA scripts? This is the automatic tests I added to my fork on GitHub. Due to a synchronisation error between master and the branch the "Add GHA scripts" commit was shown as part of the PR; I've fixed it now. A |
|
From: Günter M. <mi...@us...> - 2022-03-01 22:59:12
|
Thank you again, Adam for the work on this tricky problem. I propose a change of plan: My original naïve idea was: > After raising the Python dependency to >=3.7, now may be the > right time to make the move. In the light of the complexity and extent of the required changes, I retract the suggestion to make the move now. The danger of accidential breaks for 3rd party code is IMO too high for this change to be done without any advance warning. As there is no stated removal date/version for "optparse" we have a chance to prepare and start warnings/deprecations now and do the actual switch later. This would also solve us from the headache of re-implementing OptionParser on the basis of "ArgumentParser" and break the lock on other changes (allow to continue with code cleanup and "low hanging fruit" and prepare 0.19b). Back to the discussion: >> We have 3 settings sources: components, config files, command line: >> Are settings after updating from config files still "default" >> settings? > You could argue yes, the settings are the default for the particular > environment. Happy to take better suggestions, though. I don't have a compelling name suggsestion, unfortunately, but I am increasingly convinced that settings updated from configuration files are not "default settings". In programmatic use, one can easily differentiate "defaults" fallbacks, set by Docutils vs. "custom values", set in the configuration file (with the defaults as fallback). This is less clear in command line use (where config-file-customized values that are fed to the OptionParser/ArgumentParser as "default=..."). >> I propose a more versatile variant that allows for >> "settings_overrides" and customization of the config file names. > I see the idea, but I would very strongly advise against -- this is > meant to be a simple convinience function. If the more complex > functionality is needed later, we can always either add it to the same > function, or add a new one. I agree. So, for now future-proof access to customized settings relies on core.Publisher methods (cf. the patch for utils/__init__.py). I'll attach a diff between the first commit in "pull/8" and my suggestion together with a review. Attachments: - [argparse-aa.review](https://sourceforge.net/p/docutils/bugs/_discuss/thread/c76f9a2618/45c6/3ffa/7fbd/0cd9/attachment/argparse-aa.review) (2.2 kB; application/octet-stream) - [argparse-aa-gm.diff](https://sourceforge.net/p/docutils/bugs/_discuss/thread/c76f9a2618/45c6/3ffa/7fbd/0cd9/attachment/argparse-aa-gm.diff) (27.1 kB; text/x-patch) --- ** [bugs:#441] Move from "optparse" to "argparse".** **Status:** open **Created:** Thu Jan 06, 2022 03:02 PM UTC by Günter Milde **Last Updated:** Sun Feb 20, 2022 06:14 AM UTC **Owner:** Günter Milde The optparse documentation says: > Deprecated since version 3.2: The optparse module is deprecated and will not be developed further; development will continue with the argparse module. We are currently suppressing related deprecation warnings in the test suite. After raising the Python dependency to >=3.7, now may be the right time to make the move. --- Sent from sourceforge.net because doc...@li... is subscribed to https://sourceforge.net/p/docutils/bugs/ To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/docutils/admin/bugs/options. Or, if this is a mailing list, you can unsubscribe from the mailing list. |
|
From: Adam T. <aa-...@us...> - 2022-03-02 03:00:58
|
I've combined your changes into the attached patch, they look good to me as enabling works. > retract the suggestion to make the move now OK. It would be good to talk more about the "settings api" proposals. > I don't have a compelling name suggsestion, unfortunately, but I am increasingly convinced that settings updated from configuration files are not "default settings". > In programmatic use, one can easily differentiate "defaults" fallbacks, set by Docutils vs. "custom values", set in the configuration file (with the defaults as fallback). > This is less clear in command line use (where config-file-customized values that are fed to the OptionParser/ArgumentParser as "default=..."). Perhaps "initital" settings? A Attachments: - [0001-Preparations-for-argparse-migration.patch](https://sourceforge.net/p/docutils/bugs/_discuss/thread/c76f9a2618/45c6/3ffa/7fbd/0cd9/7a23/attachment/0001-Preparations-for-argparse-migration.patch) (27.0 kB; application/octet-stream) --- ** [bugs:#441] Move from "optparse" to "argparse".** **Status:** open **Created:** Thu Jan 06, 2022 03:02 PM UTC by Günter Milde **Last Updated:** Tue Mar 01, 2022 10:50 PM UTC **Owner:** Günter Milde The optparse documentation says: > Deprecated since version 3.2: The optparse module is deprecated and will not be developed further; development will continue with the argparse module. We are currently suppressing related deprecation warnings in the test suite. After raising the Python dependency to >=3.7, now may be the right time to make the move. --- Sent from sourceforge.net because doc...@li... is subscribed to https://sourceforge.net/p/docutils/bugs/ To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/docutils/admin/bugs/options. Or, if this is a mailing list, you can unsubscribe from the mailing list. |
|
From: Günter M. <mi...@us...> - 2022-03-02 10:17:56
|
On 2022-03-02, Adam Turner wrote:
> I've combined your changes into the attached patch, they look good to
> me as enabling works.
Thanks for look at this. We are almost there.
`python3 -W all alltests.py` came up with a buch of DeprecationMessages which
could be solved with
~~~
--- a/docutils/test/test_settings.py
+++ b/docutils/test/test_settings.py
@@ -108,8 +108,7 @@ class ConfigFileTests(unittest.TestCase):
def setUp(self):
warnings.filterwarnings(action='ignore',
category=frontend.ConfigDeprecationWarning)
- warnings.filterwarnings(action='ignore', module='docutils.frontend',
- category=PendingDeprecationWarning)
+ warnings.filterwarnings(action='ignore', category=DeprecationWarning)
self.option_parser = frontend.OptionParser(
components=(pep_html.Writer, rst.Parser), read_config_files=None)
~~~
Note: testing with warnings switched on is helpful. (It would also be
good to run one or several Sphinx builds with activated warnings,
to see whether there are uses of deprecated functions/classes.)
Now, only the entries in RELEASE-NOTES (bacwards-incompatible changes,
deprecations) and maybe HISTORY (other important changes) are missing.
...
>> I retract the suggestion to make the move now
> OK. It would be good to talk more about the "settings api" proposals.
Yes. But this may be done in parallel to (or after) releasing 0.19b, ..., 0.19,
so that the deprecation warnings are out.
>> [...] I am increasingly convinced that settings updated from
>> configuration files are not "default settings".
> Perhaps "initital" settings?
Thats an option. Best would be to find out whether there is an precedence
of projects using both, ConfigParser and ArgumentParser, how they combine
both, and how they name partially configured settings.)
---
** [bugs:#441] Move from "optparse" to "argparse".**
**Status:** open
**Created:** Thu Jan 06, 2022 03:02 PM UTC by Günter Milde
**Last Updated:** Wed Mar 02, 2022 02:40 AM UTC
**Owner:** Günter Milde
The optparse documentation says:
> Deprecated since version 3.2: The optparse module is deprecated and will not be developed further; development will continue with the argparse module.
We are currently suppressing related deprecation warnings in the test suite.
After raising the Python dependency to >=3.7, now may be the right time to make the move.
---
Sent from sourceforge.net because doc...@li... is subscribed to https://sourceforge.net/p/docutils/bugs/
To unsubscribe from further messages, a project admin can change settings at https://sourceforge.net/p/docutils/admin/bugs/options. Or, if this is a mailing list, you can unsubscribe from the mailing list. |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X