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

Value of setup.cfg [options.package_data] entry * = [...] is converted to a string representation of [...] #35

Open
bskinn opened this issue Mar 25, 2022 · 8 comments
Labels
FR Feature Request low priority

Comments

@bskinn
Copy link

bskinn commented Mar 25, 2022

First -- thank you for all your work around pyproject.toml and setuptools, and for this conversion helper -- very much appreciated!!

I'm using ini2toml 0.10 and Python 3.9.11. The following piece of my setup.cfg

[options.package_data]
* = ['extension/jupyter_tempvars.*']

is being converted to

[tool.setuptools.package-data]
"*" = [ "['extension/jupyter_tempvars.*']",]

I'm pretty sure it should just be

"*" = ['extension/jupyter_tempvars.*',]

This was the only array value in the entire setup.cfg, so I don't have any other array values to compare to. Is this parse case just not implemented yet?

@abravalheri
Copy link
Owner

Hi @bskinn thank you very much for reporting this.
What I think is happening is the following:

  • The syntax of the setup.cfg files is not really aware about arrays/lists of value. There is no way of representing this type of values in .cfg or .ini files
  • The way setuptools works for options.package_data is by accepting either a string that contains a , separator between values or a multiline string where each line is a different value.

There is an example on the setuptools website about this: https://setuptools.pypa.io/en/latest/userguide/declarative_config.html

So I don't think ini2toml is doing the wrong thing in this particular case (there might be other bugs elsewhere...).

Does this make sense to you?

@bskinn
Copy link
Author

bskinn commented Mar 25, 2022

Oh, interesting -- I didn't realize that was a limitation of the INI format. Yes, it makes total sense!

Would it make sense to add a warning, displayed to the user, if ini2toml detects values that "look like arrays", so that they would know to find and revise them?

@abravalheri
Copy link
Owner

I didn't realize that was a limitation of the INI format. Yes, it makes total sense!

That is one of the reasons why people want to move towards TOML. It is easier to work with.

Would it make sense to add a warning, displayed to the user, if ini2toml detects values that "look like arrays"

That is a bit tricky... Do you know if there is a validator for setup.cfg somewhere? Maybe that would help?

@bskinn
Copy link
Author

bskinn commented Mar 25, 2022

Do you know if there is a validator for setup.cfg somewhere? Maybe that would help?

I don't offhand, unfortunately. Perhaps there's something in the guts of setuptools that could be exploited?

That said -- perhaps a "sloppy" warning would be good enough? E.g., emitting a warning that 'an array may be present', any time a string value contains both [ and ]?

(It seems like the multiline strings are interpreted well by ini2toml, and would be of less concern... e.g., my classifiers were correctly transformed into an array.)

@bskinn
Copy link
Author

bskinn commented Mar 25, 2022

E.g., emitting a warning that 'an array may be present', any time a string value contains both [ and ]?

I guess declaring dependencies would be one time that [ and ] will be present and not represent an array... for package extras. Hm.

@abravalheri
Copy link
Owner

I don't offhand, unfortunately. Perhaps there's something in the guts of setuptools that could be exploited?

I checked... setuptools does not seem to have any kind of checks for this problem.

This can be done directly in ini2toml, although it will increase the complexity of the already complex code...

Would you like to give it a try and propose a PR?

@bskinn
Copy link
Author

bskinn commented Mar 25, 2022

Sure, I'll take a look and see if I can come up with something. Can't make any promises on a timeline, though.

@abravalheri
Copy link
Owner

Thank you very much @bskinn, there is absolutely no pressure and no rush.

This is a "nice to have", but if too complicated or if none of us has the bandwidth, we can still use the package like it is nowadays.

@abravalheri abravalheri added FR Feature Request low priority labels Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FR Feature Request low priority
Projects
None yet
Development

No branches or pull requests

2 participants