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

centrally defined abbreviations for units with special formatting #2209

Open
wants to merge 17 commits into
base: main
Choose a base branch
from

Conversation

RDaxini
Copy link
Contributor

@RDaxini RDaxini commented Sep 11, 2024

Additional context
Suggested replacements:
|wm2| -> Wm⁻²
|wm2nm1| -> Wm⁻²nm⁻¹
|degc| -> °C
|deg| -> °
Note: click shift+\ to type |

Includes some common units but the current list just gives an idea of what is possible. Feel free to recommend inclusions/exclusions/modifications. See here for example usage (solar_azimuth, dhi, dni_extra)

I also tried:
|^-1| -> ⁻¹
|^-2| -> ⁻²
to create some individual superscripts for more flexible applications (e.g. in atmosphere.pres2alt we have ms⁻²) but this did render correctly in the docs. Is this a problem / does anyone have a suggestion to resolve this?

ps ignore the branch name, nothing to do with latex

@RDaxini
Copy link
Contributor Author

RDaxini commented Sep 12, 2024

@echedey-ls do you have any ideas about this?
None of these attempts worked (apart from using :math:, but no-one wants that) 😂

I was not confident about the latex commands since the docs are html, not pdf, but I thought creating the new roles would work, or at the very least just typing out the superscript manually with <sup> </sup> would work,,, but no

@echedey-ls
Copy link
Contributor

echedey-ls commented Sep 12, 2024

I haven't had a look at your commits, but the best thing that comes to my mind is the following:

IDK at what point I've become your sphinx reference guy, but I don't dislike that hahahaha.

and reverting other changes
@RDaxini RDaxini changed the title [draft] (trying to) define units commands via latex/custom roles [draft] centrally defined abbreviations for units with special formatting Sep 13, 2024
@echedey-ls
Copy link
Contributor

echedey-ls commented Sep 13, 2024

I think this PR is now a nice example (thx Dax) of how this would look like: https://pvlib-python--2209.org.readthedocs.build/en/2209/reference/generated/pvlib.irradiance.get_total_irradiance.html

Params dhi and dni_extra.

@RDaxini
Copy link
Contributor Author

RDaxini commented Sep 13, 2024

@echedey-ls credit to you for suggesting the substitutions method! I was not aware of that functionality in sphinx.
(there you go, this is why you're the sphinx reference guy 😂)

@RDaxini RDaxini mentioned this pull request Sep 15, 2024
6 tasks
@RDaxini
Copy link
Contributor Author

RDaxini commented Sep 16, 2024

conf.py seems to have a lot of linting issues, for example # not being followed by a space, which I guess in those cases is since the # is followed by an optional line of code. How did conf.py pass the Flake8 Linter test in the past? Should I resolve these linting issues or leave them?

@RDaxini RDaxini marked this pull request as ready for review September 16, 2024 12:11
@RDaxini RDaxini changed the title [draft] centrally defined abbreviations for units with special formatting centrally defined abbreviations for units with special formatting Sep 16, 2024
@RDaxini RDaxini closed this Sep 16, 2024
@RDaxini RDaxini deleted the latex_units_test branch September 16, 2024 12:20
@RDaxini RDaxini restored the latex_units_test branch September 16, 2024 12:20
@RDaxini RDaxini reopened this Sep 16, 2024
@kandersolar
Copy link
Member

How did conf.py pass the Flake8 Linter test in the past?

The flake8 job is configured to only check lines that are near other lines that have changed in the PR, and we don't change conf.py very often :)

Should I resolve these linting issues or leave them?

My two cents: reviewing PRs is already hard enough without extra "clutter" of unrelated formatting changes. Best to do them in a separate PR where improved formatting is the objective.

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.

Standarizing pvlib typesetting of units - standarization for common variables?
3 participants